Also disable Sonar when TSLint rule is disabled inline

[xxluke]

I want to request a feature.
We use tslint-sonarts on the project before we check it with Sonar. When there is an issue we won't fix, we disable the lint rule by adding a tslint:disable comment. But then it's still active in Sonar, even it's the same rule:

Of course I could also add a // nosonar, but it's extra work which seems unneccessary and it's annoying since we don't notice the problem until we perform the whole analysis (we can't use the IDE plugin).

This might be a missing feature of Sonar in general, not limited to TypeScript.

[vilchik-elena]

@xxluke I don't think we want to consider tslint disable comments in SonarQube
In your use-case, if you really want to use TSLint, I would suggest to have an empty profile in SonarQube for TS and import tslint report. That way you will see only tslint issues left after tslint:disable. See docs on this feature here.

[morris]

The bigger issue here is that // nosonar ignores all errors on a line, e.g.

if (4 === 4) { // nosonar

will not report any issues, as opposed to

if (4 === 4) { // tslint:disable-line:no-magic-numbers

where the no-identical-expressions will still be reported.

@vilchik-elena that would make SonarQube obsolete for us (we could just tslint on pre-push instead), or am I missing something? In any case, the feature described by @xxluke would be extremely helpful IMHO :)

[vilchik-elena]

@Marris The idea behind SonarQube is that you mark issue as "won't fix" or "FP" in the SQ UI, so that you are not bothered by it in future. Usage of NOSONAR is not encouraged, so I don't think that any soon we will evolve it to support rule key.
SonarQube can bring much more other value compared to TSLint (metrics, quality gate, leak period, history, coverage, branches, many languages etc) but if you believe that for your project the main interest is to have 0 issues, I think indeed TSLint will be a more appropriate.

[morris]

SonarQube can bring much more other value compared to TSLint (metrics, quality gate, leak period, history, coverage, branches, many languages etc) but if you believe that for your project the main interest is to have 0 issues, I think indeed TSLint will be a more appropriate.

Definitely, that's why we are using it in the first place :) we haven't really been using the UI workflow that much yet, but if that's an encouraged workflow it could work for us.

I still believe the requested feature is an enhancement worth looking at from SQ's perspective. I don't see any downside right now, and it would certainly make SQ's analysis more accurate.

[melgish]

// Disabled rule because the third-party API chokes on anything else.
// tslint:disable-next-line:any-rule-you-want
const runsWithScisors: any = 3;

Would it be that much more difficult to support both? Have an option in SQ to ignore (the default) or respect the tslint:disable comments in source. This way everyone is happy. The existence of 'won't-fix' means accept that there will be times when rules will be broken out of necessity. However a 'won't-fix' in sonar is out-of-sight, out-of-mind when the developer is in the code. However the above comment makes it clear somebody considered the rule, but had to reject it.

2020-02-03 update: A follow up because I'm annoyed :-) We have some obsolete API calls in a library module that we marked with @deprecated, intending to remove with the next major release. We added disable-next-line to the unit tests for the API so that we didn't get any lint errors. We still need tests.

Our subcontractor did some work in the library and while doing so, they "fixed" all the SQ reported smells by deleting all of the deprecated methods.

Fortunately I caught this before a release was cut, but it does illustrate a real life use case for providing the feature.