Skip to content

Commit d9b7fd6

Browse files
pynicolaslindamartin
pynicolas
authored and
lindamartin
committed
SONARJS-515 Rule: Results of operations on strings should not be ignored
1 parent ef05d14 commit d9b7fd6

8 files changed

Lines changed: 148 additions & 1 deletion

File tree

‎its/ruling/src/test/expected/javascript-S1154.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
'project:dojo-1.8.1/dijit/dijit-all.js.uncompressed.js':[
3+
23138,
4+
],
5+
'project:dojo-1.8.1/dijit/form/ValidationTextBox.js.uncompressed.js':[
6+
285,
7+
],
8+
}

‎its/ruling/src/test/profile.xml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,5 +609,10 @@
609609
<key>S3002</key>
610610
<priority>INFO</priority> <!-- Unary operators "+" and "-" should not be used with objects -->
611611
</rule>
612+
<rule>
613+
<repositoryKey>javascript</repositoryKey>
614+
<key>S1154</key>
615+
<priority>INFO</priority><!-- Results of operations on strings should not be ignored -->
616+
</rule>
612617
</rules>
613618
</profile>

‎javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public static List<Class> getChecks() {
142142
NewOperatorMisuseCheck.class,
143143
WebSQLDatabaseCheck.class,
144144
PostMessageCheck.class,
145+
UselessStringOperationCheck.class,
145146
LocalStorageCheck.class,
146147
UnaryPlusMinusWithObjectCheck.class,
147148
BackboneChangedIsUsedCheck.class,

‎javascript-checks/src/main/java/org/sonar/javascript/checks/UselessStringOperationCheck.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* SonarQube JavaScript Plugin
3+
* Copyright (C) 2011 SonarSource and Eriks Nukis
4+
* [email protected]
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public
17+
* License along with this program; if not, write to the Free Software
18+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
19+
*/
20+
package org.sonar.javascript.checks;
21+
22+
import com.google.common.collect.ImmutableList;
23+
import org.sonar.api.server.rule.RulesDefinition;
24+
import org.sonar.check.Priority;
25+
import org.sonar.check.Rule;
26+
import org.sonar.javascript.checks.utils.CheckUtils;
27+
import org.sonar.plugins.javascript.api.symbols.Type;
28+
import org.sonar.plugins.javascript.api.tree.Tree;
29+
import org.sonar.plugins.javascript.api.tree.Tree.Kind;
30+
import org.sonar.plugins.javascript.api.tree.expression.CallExpressionTree;
31+
import org.sonar.plugins.javascript.api.tree.expression.ExpressionTree;
32+
import org.sonar.plugins.javascript.api.tree.expression.MemberExpressionTree;
33+
import org.sonar.plugins.javascript.api.tree.statement.ExpressionStatementTree;
34+
import org.sonar.plugins.javascript.api.visitors.SubscriptionBaseTreeVisitor;
35+
import org.sonar.squidbridge.annotations.ActivatedByDefault;
36+
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
37+
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;
38+
39+
import java.util.List;
40+
41+
@Rule(
42+
key = "S1154",
43+
name = "Results of operations on strings should not be ignored",
44+
priority = Priority.BLOCKER,
45+
tags = {Tags.BUG})
46+
@ActivatedByDefault
47+
@SqaleSubCharacteristic(RulesDefinition.SubCharacteristics.INSTRUCTION_RELIABILITY)
48+
@SqaleConstantRemediation("20min")
49+
public class UselessStringOperationCheck extends SubscriptionBaseTreeVisitor {
50+
51+
@Override
52+
public List<Kind> nodesToVisit() {
53+
return ImmutableList.of(Kind.EXPRESSION_STATEMENT);
54+
}
55+
56+
@Override
57+
public void visitNode(Tree tree) {
58+
Tree expression = ((ExpressionStatementTree) tree).expression();
59+
if (expression.is(Kind.CALL_EXPRESSION)) {
60+
ExpressionTree callee = ((CallExpressionTree) expression).callee();
61+
if (callee.is(Kind.DOT_MEMBER_EXPRESSION)) {
62+
MemberExpressionTree memberExpression = (MemberExpressionTree) callee;
63+
if (memberExpression.object().types().containsOnly(Type.Kind.STRING)) {
64+
String variableName = CheckUtils.asString(memberExpression.object());
65+
addIssue(tree, variableName + " is an immutable object; you must either store or return the result of the operation.");
66+
}
67+
}
68+
}
69+
}
70+
71+
}

‎javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1154.html‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<p>Doing an operation on a string without using the result of the operation is useless and is certainly due to a misunderstanding. </p>
2+
<h2>Noncompliant Code Example</h2>
3+
4+
<pre>
5+
var str = "..."
6+
str.toUpperCase(); // Noncompliant
7+
</pre>
8+
<h2>Compliant Solution</h2>
9+
10+
<pre>
11+
var str = "..."
12+
str = str.toUpperCase();
13+
</pre>
14+

‎javascript-checks/src/test/java/org/sonar/javascript/checks/UselessStringOperationCheckTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* SonarQube JavaScript Plugin
3+
* Copyright (C) 2011 SonarSource and Eriks Nukis
4+
* [email protected]
5+
*
6+
* This program is free software; you can redistribute it and/or
7+
* modify it under the terms of the GNU Lesser General Public
8+
* License as published by the Free Software Foundation; either
9+
* version 3 of the License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
* Lesser General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Lesser General Public
17+
* License along with this program; if not, write to the Free Software
18+
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
19+
*/
20+
package org.sonar.javascript.checks;
21+
22+
import org.junit.Test;
23+
import org.sonar.plugins.javascript.api.tests.TreeCheckTest;
24+
import org.sonar.squidbridge.checks.CheckMessagesVerifier;
25+
26+
public class UselessStringOperationCheckTest extends TreeCheckTest {
27+
28+
@Test
29+
public void test() {
30+
UselessStringOperationCheck check = new UselessStringOperationCheck();
31+
CheckMessagesVerifier.verify(getIssues("src/test/resources/checks/UselessStringOperation.js", check))
32+
.next().atLine(4).withMessage("str is an immutable object; you must either store or return the result of the operation.")
33+
.next().atLine(5).withMessage("\"abc\" is an immutable object; you must either store or return the result of the operation.")
34+
.next().atLine(6)
35+
.noMore();
36+
}
37+
38+
}

‎javascript-checks/src/test/resources/checks/UselessStringOperation.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
var var1 = "abc".toUpperCase(); // OK
2+
unknown.toUpperCase(); // OK
3+
var str = "abcd";
4+
str.toUpperCase(); // NOK
5+
"abc".toUpperCase(); // NOK
6+
str.substring(1,2); // NOK
7+
8+
var x = "abc";
9+
x = something();
10+
x.toUpperCase(); // OK

‎sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptProfileTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public void should_create_sonar_way_profile() {
4646
assertThat(profile.getLanguage()).isEqualTo(JavaScriptLanguage.KEY);
4747
assertThat(profile.getName()).isEqualTo(CheckList.SONAR_WAY_PROFILE);
4848
assertThat(profile.getActiveRulesByRepository(CheckList.REPOSITORY_KEY))
49-
.hasSize(77);
49+
.hasSize(78);
5050
assertThat(validation.hasErrors()).isFalse();
5151
}
5252

0 commit comments

Comments
 (0)