Skip to content
This repository was archived by the owner on Mar 25, 2021. It is now read-only.

Allow rules requiring type checking to silently fail if you don't have type checking enabled #1461

Closed
glen-84 opened this issue Aug 7, 2016 · 11 comments

Comments

@glen-84
Copy link
Contributor

glen-84 commented Aug 7, 2016

This would allow you to see all the other errors/warnings in an editor, even when you haven't enabled type checking (which won't always be possible or easy to do [see https://github.com/Microsoft/vscode-tslint/issues/70]).

If silent failure is bad, you could warn unless a configuration option is enabled that suppresses these warnings.

@jkillian
Copy link
Contributor

jkillian commented Aug 8, 2016

cc @ScottSWu, thoughts?

@ScottSWu
Copy link
Contributor

ScottSWu commented Aug 8, 2016

Warnings sounds alright. Would it somehow be encoded as a LintFailure (which would show up once per file per rule)? And then a suppressWarnings option for linterOptions?

@glen-84
Copy link
Contributor Author

glen-84 commented Aug 8, 2016

Would it be possible to suppress these specific warnings via the tslint.json file? Something like:

{
    "options": {
        "suppressWarnings": [
            "typeCheckingNotEnabled"
        ]
    },
    "rules": {

    }
}

@jkillian
Copy link
Contributor

jkillian commented Aug 9, 2016

I like the idea of making it a lint failure, this was it would interact nicely with an IDE (as opposed to a thrown error, which is a bad experience in a situation which isn't necessarily a user error). It's not really something we've done before though... For example, an invalid rule name also throws an error right now.

Perhaps we should add a linterOptions field to tslint.json and it should mirror the command-line flags? We need to think carefully about what configuration values go where, see #73 (comment) as well

@glen-84
Copy link
Contributor Author

glen-84 commented Aug 9, 2016

I wouldn't be able to use the tslint.json file for the project option, because we install this file globally (it's shared between multiple projects).

In our case, we just need a way to suppress these (specific) warnings from the config file, because we can't guarantee that every member on the team has a correctly-configured editor extension.

@ScottSWu
Copy link
Contributor

ScottSWu commented Aug 9, 2016

It sounds like linterOptions mirroring the command line would either have redundant code or break the Linter API. But it would allow something like,

if (rule instanceof TypedRule) {
    if (this.program) {
        ruleFailures = rule.applyWithProgram(sourceFile, this.program);
    } else if (!this.options.configuration.suppressSomething) {
        throw new Error(`${rule.getOptions().ruleName} requires type checking`);
    }
} else {
    ruleFailures = rule.apply(sourceFile);
}

Alternatively returning a rule failure:

return [new RuleFailure(sourceFile, -1, -1,
    "this rule requires type checking", this.getOptions().ruleName)];

now seems sort of hacky - it'll show up in the output but not in vscode.

@jkillian
Copy link
Contributor

Hmm, agreed, the RuleFailure method does seem weird.

If we modify how configs work I think we'll have to:

This seem reasonable? Have any better ideas?

@jkillian
Copy link
Contributor

Sidenote: I'm not against doing a major release soon if we need to make larger changes to how configuration works.

@ScottSWu
Copy link
Contributor

Will the linter options need to be computed in tslint-cli? Wrt type checking, the program must be created outside Linter in order to know all the source files, but the configuration is parsed inside.

@jkillian
Copy link
Contributor

I'm not sure of the best way to handle this yet, but relatedly, there's some parallel work over in #1472 right now, which defines a new class for linting multiple files at a time.

@glen-84
Copy link
Contributor Author

glen-84 commented Jul 2, 2017

@jkillian Should this be closed because of #2188?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants