jsRules:true does not copy over explicitly disabled rules

[UselessPickles]

Bug Report

• TSLint version: 5.12.1
• TypeScript version: 3.2.4
• Running TSLint via: CLI

TypeScript code being linted

(any JS file)

with tslint.json configuration:

{
    "jsRules": true,
    "rules": {
        // any JS-compatible rule that is enabled by default?, disabled
        "object-literal-sort-keys": false    
    }
}

Actual behavior

Default enabled rules are still applied to JS files, even when explicitly disabled in "rules" and "jsRules": true is used.

In this PR: https://github.com/palantir/tslint/pull/3641/files

The implementation of filterValidJsRules only copies over JS-compatible rules if ruleOptions.ruleSeverity !== "off".

This seems like an odd intentional behavior. I think simply removing that if check will produce expected behavior, but I'm hesitant because it appears that this was intentionally done for some reason. Is there a good reason that disabled rules are ignored?

Expected behavior

Configuration of all JS-compatible rules should be copied over to and applied to JS files, including those that are explicitly disabled.

[UselessPickles]

@mlavina - As the original PR author, can you provide any insight into why explicitly disabled rules are intentionally ignored when copying over to jsRules?

[mlavina]

@UselessPickles my best guess was that it was because that was my best guess for the issue here #2198

Which specifically states Active rules.

[mlavina]

Though I don't have a strong opinion either way haha

[UselessPickles]

Hmmm... is there a distinction between a "non-active" rule and a rule that is explicitly disabled via the config.

I wonder if the original intent/desire was to copy over all "specified" rules, rather than all "active" rules?

[UselessPickles]

Either way, I feel like my expectations are quite reasonable: however I configured the "rules", I expect it to be also be applied to JS files when "jsRules": true. I can't think of a good reason for a rule that I explicitly configured as disabled should be exempt from this behavior.

If someone with authority can make the decision, I will gladly create a PR for this.

[adidahiya]

@UselessPickles I think your expectations are reasonable, it might end up being a breaking change but I'm open to PRs for this

[retwedt]

+1 just ran into this issue today. While it is something we can work around, it wasn't the behavior I was expecting. Thanks for your help!

[UselessPickles]

PR submitted: #4517

[adidahiya]

fixed by #4517