Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/53 always show disable rule fix #87

Closed
wants to merge 2 commits into from
Closed

Feature/53 always show disable rule fix #87

wants to merge 2 commits into from

Conversation

reduckted
Copy link

Fixes #53.

These changes allow the "disable rule for the next line" fix to be shown when there are no other fixes available.

Basically, instead of only recording a failure if it can be fixed, the failure is always recorded along with a boolean that indicates whether it can be fixed. Then, anywhere that was previously looking at the recorded failures (which were always fixable) will filter the failures so that it only uses the fixable failures. The only exception to this is when adding the "disable rule for the next line" fix. It will add the fix regardless of whether the failure is fixable.

I've also renamed any variables called problem to failure to avoid any confusion about whether it's a tslint.Failure or the new Problem interface I created, which is why there seems to be a lot of changes.

@angelozerr
Copy link
Owner

@egamma what do you think?

@arogg
Copy link

arogg commented Oct 24, 2018

Thanks @reduckted for this , i am using your fix right now (regardless if its good or not, I just needed it), but if a line has 2 problems, I can't disable both problems (only the last comment goes into effect and the fix suggestion thus keeps alternating). Example: (requirePath is any)

// tslint:disable-next-line:variable-name
// tslint:disable-next-line:no-unsafe-any
// tslint:disable-next-line:non-literal-require
// tslint:disable-next-line:no-unsafe-any
// tslint:disable-next-line:non-literal-require
// tslint:disable-next-line:no-unsafe-any
// tslint:disable-next-line:non-literal-require
// tslint:disable-next-line:no-unsafe-any
const Plugin = require(requirePath) as RuntimePluginConstructor<PluginTypes<WebProjectType>>;

If this problem is yours you should fix it, otherwise great work :D

@reduckted
Copy link
Author

reduckted commented Oct 26, 2018

Hmm, that looks like it might be a bug in TSLint. Here's the same problem using the VS Code extension instead of the language service:

disabled

Edit: Actually, it looks like it is a bug in this package (it just happens to be the same bug that the VS Code extension has), but it's an existing bug that would happen even without the changes in this PR.

@egamma
Copy link
Contributor

egamma commented Oct 26, 2018

@reduckted actually the tslint plugin and vscode-tslint share code and share this limitation, so this is not a tslint issue.

// FYI @mjbvz

@reduckted
Copy link
Author

Closing this since TSLint has been deprecated.

@reduckted reduckted closed this Aug 14, 2019
@reduckted reduckted deleted the feature/53-always-show-disable-rule-fix branch August 14, 2019 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show the disable-next-line fix always.
4 participants