sonarsource/sonarjs

SonarSource Static Analyzer for JavaScript and TypeScript

This SonarSource project is a static code analyzer for the JavaScript, TypeScript, and CSS languages to produce Clean code.

➡️ Have some feedback?

➡️ eslint-plugin-sonarjs, our plugin for ESLint

Features

• Advanced rules based on pattern matching and control flow analysis
• 397 JS rules and 402 TS rules
• 26 CSS rules
• Compatible with ECMAScript 2015-2020
• React JSX, Flow, Vue, and AWS lambda functions support for JavaScript and TypeScript
• CSS, SCSS, SASS, Less, also 'style' inside HTML and VueJS files
• Metrics (complexity, number of lines, etc.)
• Import of test coverage reports
• Import of ESLint, TSLint, and Stylelint issues

Documentation

You can find documentation here

Have question or feedback?

SonarSource Community Forum

If you want to report a bug, request a feature, or provide other kind of feedback, please use SonarQube Community Forum. Please do not forget to specify the details of your request, code reproducer, and versions of projects you use.

Contributing

1. Request a new feature

To request a new feature, create a new thread in SonarSource Community Forum. Even if you plan to implement it yourself and submit it back to the community, please create a thread to be sure that we can follow up on it.

2. Pull Request

To submit a contribution, create a pull request for this repository. Please make sure that you follow our code style and that all tests are passing.

Work with us

Would you like to work on this project full-time? We are hiring! Check out https://www.sonarsource.com/hiring

License

Copyright 2011-2023 SonarSource.

Licensed under the GNU Lesser General Public License, Version 3.0

sonarjs's Issues

S930 misinterpret class getter as function calls

I want to report a bug.

SonarJS version: 2.21 (build 4409)
SonarQube version: 6.3.1 (build 21392)

Rule key: javascript:S930

Reproducer

class {
  /**
   * @return {Function}
   */
  get callback() { 
    return this._callbackFn;
  }

  executeCallback(param) {
    this.callback(param);
  }
}

I'm getting the error: "callback" expects 0 arguments, but 1 was provided.

Expected behavior
There shouldn't be any error as the callback getter can return a function.

Add rule S4125: "typeof" expressions should only be compared to valid values

Implements RSPEC-4125)

Can I use sonar-javascript as a standalone tool?

Deploy a sonar server and make sonar-javascript as a plugin of that which is too heavy for me.

Is there any hack I can do to extract the check logic from the source to make it as a standalone cli tool?

Thanks.

Support wildcards for coverage report paths

Currently property for coverage LCOV report paths doesn't accept paths with wildcards. This might be very useful when path is not stable (e.g. see question on SOF).

Property name: sonar.javascript.lcov.reportPaths

• Docs for coverage import in SonarJS
• Docs for patterns in SonarQube

We should decide whether this feature should be implemented for old properties (still supported for SQ < 6.2):

• sonar.javascript.lcov.reportPath
• sonar.javascript.lcov.itReportPath

Example:
"sonar.javascript.lcov.reportPaths=reports/*" would match
reports/report1.lcov
reports/report2.lcov

"sonar.javascript.lcov.reportPaths=**/report.lcov" would match
reports1/report.lcov
reports2/report.lcov

Rule: "for of" should be used with Iterables

Implements RSPEC-4138

Rule: Collection sizes and array length comparisons should make sense

Implements RSPEC-3981
common rule

Port #615 to master

Apply #615 to master

Update message for ReturnInSetterCheck (S2432)

RSPEC-2432
Make message for informative

Support Vue.js Single File Components

I want to request a feature.

Hi, is there any plan to support .vue file?

Rule OneStatementPerLine: add parameter for toggling exceptions back off

RSPEC-122
Version 3.0 added exceptions in this rule (ticket SONARJS-944) to ignore things like

if (condition) doTheThing;  // ignored by exception
var a = 0; doTheOtherThing;  // Still noncompliant

But many people who turn this rule on will want to find all instances of multiple statements on a line, especially the ones after conditions or loops. So a parameter should be added to the rule to allow the user to disable these exceptions.

Rule: Deprecated features should not be used

Implements RSPEC-3817

javascript:DuplicatePropertyName also detects static/nonstatic class functions

I want to report a bug.

SonarJS version: 2.21 (build 4409)
SonarQube version: 6.3.1 (build 21392)

Rule key: javascript:DuplicatePropertyName

Reproducer

class {
  myFunc() {  }

  static myFunc() { }
}

I'm getting the error: Rename or remove duplicate property name 'myFunc'.

Expected behavior
Well it may not be a good style to have 2 functions with the same name but as they exists in different scopes I would expect a different error message.

S3827 should catch non-existent variables used in export statements

RSPEC-3827
Reproducer

export { a }  // 'a' is undefined and S3827 should thus raise an issue here

This was originally raised in #621

GetterSetterCheck should not fail on duplicate getter

Migrated from SONARJS-904
GetterSetterCheck (RSPEC-2376) should not fail on:

class C {
  get p1() { return 42; }
  get p1() { return 42; }
}

Safe usage of localStorage

I had a look on the JS rules filtered on localStorage and didn't find anything like what I propose below.

DOM storages may be broken at different levels.

The existing cases I know of:

• sessionStorage and localStorage are null (chrome and firefox when DOM storage is disabled by configuration)
• localStorage is erased after the session (chrome specific configuration)
• localStorage.setItem may throw an error (Safari in private mode or quota exceeded)
• localStorage is "disabled" and invoking it throws an "Access Denied" error

Directly using localStorage is code smell. Sonar should help by highlight this problem as this may have unexpected consequences.

When the usage of localStorage is not in a try/catch statement and it throws an Error, the rest of the JS file is ignored which may break the site completely although only cross-session DOM storage is broken.

My propositions for detection rules :

• all localStorage mentions should be fenced in a try/catch
• all sessionStorage usages should be fenced in a try/catch

Exemple bad code:

/*
 * the following throws "Access denied" on IE under specific configuration
 */
var storage = localStorage; 

/*
 * the following throws:
 * - "Quota exceeded"
 * - trying to call setItem on null (disabled DOM storage)
 * - Error 22 ? on safari in private mode (by memory)
 */
storage.setItem('key', 'value'); // throws

Correct implementation:

try {
  var storage = localStorage;
  storage.setItem(...)
} catch (e) {
  // handle the different error if required
}

More information on the IE specific issue:

Usually in corporate environment, Windows may be "badly" configured and break localStorage, see https://msdn.microsoft.com/en-us/library/bb250462(v=vs.85).aspx.

What do you think ?

Improve NonExistentAssignmentOperatorCheck

Improve RSPEC-2757 : raise an issue in case of =!.
Small note : the message should not be the same used for =+ and =- since the correct operator is probably not an assignment, but a not equals !=.

Rule key: S2757

Rule: "Array.reverse" should not be used misleadingly

Implements RSPEC-4043

Consider Flow type annotations in Data Flow Analysis

See #584

Update S1515: update message and add secondary location

RSPEC-1515
Update message so that it gives more details when its's really dangerous to declare function inside loop.

FP S3516 InvariantReturnCheck

RSPEC-3516

function foo() {
  var a = foobar - 1;
  var d;

  if (cond1) {
    if (cond2) {
      d = a;
    } else {
      d = a - 1;
    }
  } else {
    if (cond3) {
      return a;
    }
    if (cond4) {
      d = a - 1;
    } else {
      d = a;
    }
  }
  return d; // this line got executed by DFA only once since program states are considered equal (in all cases 'd' is number)
}

Improve Dead Store to ignore never-read symbols

RSPEC-1854 overlaps with never used variables from RSPEC-1481

LoopsShouldNotBeInfiniteCheck: infinite loop

function foo() {
    for (let x=0 ; ; ) {       // makes rule to run infinitely
    }
}

S2583: FP with multiple if-else clauses involving NaN

RSPEC-2583 - the following code triggers FP on the third condition -

function compare1(a, b){
	return a < b ? -1 : a > b ? 1 : a >= b ? 0 : NaN;
}

https://sonarcloud.io/project/issues?id=kamushkin%3Asonarqube-tests&issues=AV0H2uTRI1Qj-tYTbHFX&open=AV0H2uTRI1Qj-tYTbHFX

Same when the code is written as if-else statements -

function compare2(a, b){

	if (a < b){
		return -1;
	}
	else if (a > b){
		return 1;
	}
	else if (a >= b){
		return 0;
	}
	else {
		return NaN;
	}
}

https://sonarcloud.io/project/issues?id=kamushkin%3Asonarqube-tests&issues=AV0H2uTTI1Qj-tYTbHFY&open=AV0H2uTTI1Qj-tYTbHFY

original source -
https://sonarcloud.io/project/issues?id=d3&open=AVcREIlyzjiM7eGZNOsO&resolved=false&types=BUG

The FP is not triggered when the third comparison is ==, so probably a bug.

Change contract of rule S1226 to make it become more relevant

This rule currently triggers an issue as soon as a method parameter is reassigned. With updated RSPEC-1226 this rule is supposed to raise an issue if an only if a method parameter, caught exception or foreach variable is reassigned without having been read before.

To be noted that this rule must be part of Sonar way and becomes a bug detection rule.

Update issue tracking url

We should update issue tracking value from https://jira.sonarsource.com/browse/SONARJS to GitHub Issues.
It's shown in the update center of SQ.

Rule: Functions should not have identical implementations

Implements RSPEC-4144
Common rule

Update "too many lines of code" rules to use LoC

RSPEC-104: Files should not have too many lines
RSPEC-138: Functions should not have too many lines

Improve IndexOfCompareToPositiveNumberCheck: ignore strings

Most of the time using indexOf with string object is safe, as checked substring is for sure not a first part, e.g. url.indexOf("?") > 0
So we should limit this rule with just array usages.
Rule could be added to SW profile when ticket is fixed

RSPEC-2692

Is SonarJs has a rule like eslint's no-undef?

http://eslint.org/docs/rules/no-undef

Regards.

S881: FP with decrement operator in loops or conditions

RSPEC-881

It is a very common and simple pattern to test directly the result of the decrement operator.

while (idx--) {
  // ...
}

if (--a) {
  // ...
}

It would therefore be best to exclude such cases from the rule.

Support parsing of Flow annotations

Migrated from: https://jira.sonarsource.com/browse/SONARJS-828
Flow: https://flow.org/

Rule S3973: A conditionally executed single line should be denoted by indentation

Implements RSPEC-3973

add '.jsx' and '.vue' file suffixes as default

Relates to #576

Note that as soon as this is done, performance IT should fail (more files will be parsed).
Also we could remove suffixes configuration from ruling IT.

Feed Cognitive Complexity metric

See SONARJS-914

Rule: Printf-style format strings should not lead to unexpected behavior at runtime

Implements RSPEC-2275

This support for printf-style comes with https://github.com/alexei/sprintf.js

Dirty issues list after SeCheck throws exception

SonarJS version: Since 3.0-RC1

Rule key: Any SeCheck rule

The strengthening of the analyzer against unexpected exceptions (see SONARJS-970) has introduced a side-effect : the SeCheck, base class for all our Data Flow Analysis checks, contains an instance field that stores all issues raised during analysis and returns them to the Sensor on SeCheck.scanFile. In case of an exception raised by a SeCheck that specific check's issues list never gets cleaned up, since the exception management happens much above and the next SeCheck.scanFile happens only when the next file is being analyzed.
If we are lucky and the next file is shorter than the previous one, a new exception is likely raised when issues collected in the previous file are saved with an higher line number that is larger than the current file lines (save issue at line 400 when file has only 300 lines). If instead we are not lucky, the issue from the previous file gets saved on the new file, showing issues that make no sense to the user and which are almost impossible to point back to this case (something like : why is SonarQube showing a "useless increment" issue on the second and third letter of a function name??).

Clear stack of parenthesized update expressions

in Data Flow Analyss, assignment program points (not an actual object yet, just a line in ExecutionStack ) leave the assigned value in the stack for assignment chaining.
When this assignment appears in a for statement (like in an update expression) we clean up the stack before proceeding.
If the assignment expression is wrapped into a parenthesized expression isProducingUnconsumedValue is wrongly returning false because it's not finding FOR_STATEMENT as parent.

Reproducer:

  for(let k = 0; ; (bar())) {
    foo();
  }

  for((k = 0); ; bar()) {
    foo();
  }

ERROR - Unable to parse file ####.jsx

Below is my sonar-project.properties code. I have to check three folders namely server,dist and webclient

sonar.projectKey=Adapt27
sonar.projectName=Adapt
sonar.projectVersion=1.0
sonar.javascript.file.suffixes=.js,.jsx
sonar.sources=server,dist,webclient

After running the sonar-scanner it checks only the js files, I need to check the .jsx files also. But it can't.

Inconsistent program state with loops and strict equality

The following code produces an inconsistent program state that results in the if condition resulting in neither a true branch nor a false branch (see the comment in the code)

function main(size) {
    var j;
    while (cond) {
        for (j = 0; j < size; j++) {
            if (target) {
                break;
            }
        }

        if (j === size) { // PS: size=0, j=zero, j < size
            foo();
        }
    }
}

Update message of S930 (TooManyArgumentsCheck): include declaration line number

RSPEC-930
Currently it's hard to find declaration of the function referenced by this rule. There is secondary location highlighted in SQ UI, but it doesn't help much if declaration is far from function call. It would be nice to include in the issue message line number where called function was declared.

Rule: Conditionals should start on new lines

Implements RSPEC-3972

Support the analysis of JS code snippets within any html files (Polymer, JSP, PHP, JSF, ...)

I want to request a feature.
Out project based on Polymer, which only have html files

Rule: Sparse arrays should not be declared

Implements RSPEC-4140

Improve AssignmentWithinCondition: allow parenthesised assignment as arrow function body

Migrated from SONARJS-998

RSPEC-1121
Such code is considered more readable with parentheses, thus it should not trigger the issue

var arrowFunction = (a) => (arr[a] = 42);

Rule: Method returns should not be invariant

I want to request a feature.
I would like to have SonarJS raise an issue when a function returns always the same value (RSPEC-3516):

function foo() {  // Noncompliant {{This function always returns the same value}}
  if(something) return 1;
  return 1;
}

A few notes about the implementation from @benzonico

• Not raising issue on method returning void and constructors (thanks captain obvious !)
• Limit raising of those issue to method with at least two return statements (that avoid getters and tons of small methods that would just make noise otherwise).
• Raise issue when all execution path end up returning SV with the same precise constraint (meaning that constraint is Unfefined/Null/Zero/true/false/... otherwise we can't know if we returned something not null if the value was really invariant).
• Raise issue when the same SV is returned for immutable types (because otherwise the internal state of the object represented by this SV can have been modified).

Rule: "for in" should not be used with iterables

Implements RSPEC-4139

ClassNotFoundExecption in CommaOperatorInSwitchCaseCheck

I want to report a bug.

SonarQube Scanner crashes with a ClassNotFoundException if a case clause consists of a function call and logical OR.

SonarJS version: 2.21 (build 4409)

SonarQube version: 6.3

SonarLint version: -

Gradle version: 3.4.1

SonarQube Scanner for Gradle version: 2.3

Rule key: S3616 (CommaOperatorInSwitchCaseCheck)

Reproducer

switch (true) {
  case true:                   // OK
  case true || false:          // OK
  case f(x):                   // OK
  case f(x) || x:              // Exception
    break;
}

Logs

$ gradle sonarqube
Starting a Gradle Daemon (subsequent builds will be faster)
Download https://plugins.gradle.org/m2/org/sonarsource/scanner/gradle/sonarqube-gradle-plugin/2.3/sonarqube-gradle-plugin-2.3.pom
Download https://plugins.gradle.org/m2/org/sonarsource/scanner/gradle/sonarqube-gradle-plugin/2.3/sonarqube-gradle-plugin-2.3.jar
:sonarqube
[...]
* What went wrong:
Execution failed for task ':sonarqube'.
> org/sonar/api/internal/google/common/collect/ImmutableList
[...]
* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':sonarqube'.
[...]
Caused by: java.lang.NoClassDefFoundError: org/sonar/api/internal/google/common/collect/ImmutableList
        at org.sonar.javascript.checks.CommaOperatorInSwitchCaseCheck.orExpressionOperands(CommaOperatorInSwitchCaseCheck.java:101)
        at org.sonar.javascript.checks.CommaOperatorInSwitchCaseCheck.visitCaseClause(CommaOperatorInSwitchCaseCheck.java:49)
        at org.sonar.javascript.tree.impl.statement.CaseClauseTreeImpl.accept(CaseClauseTreeImpl.java:91)
[...]
Caused by: java.lang.ClassNotFoundException: org.sonar.api.internal.google.common.collect.ImmutableList
[...]

Expected behavior

Analysis should not crash with an Exception. Instead an issue for rule S3616 should be reported.

Improve UseOfEmptyReturnValueCheck: ignore await

Rule key: S3699 (UseOfEmptyReturnValueCheck)

Hi,

We currently have false positives with the UseOfEmptyReturnValueCheck rule when using the await keyword.

For instance:

const doSomethingAsync = async () => {
  // ...
}

await doSomethingAsync() // UseOfEmptyReturnValueCheck error

I think that the async keyword should be excluded from the check.

Rule: Exception should not be created without being thrown

Implements RSPEC-3984.

Detecting the pattern "new xxxxxError(....);" should be enough in JS.

Rule: Map values should not be replaced unconditionally

Implements RSPEC-4143
common rule