Skip to content

Commit

Permalink
SONARJS-515 Rule: Results of operations on strings should not be ignored
Browse files Browse the repository at this point in the history
  • Loading branch information
pynicolas authored and lindamartin committed Aug 24, 2015
1 parent ef05d14 commit d9b7fd6
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 1 deletion.
8 changes: 8 additions & 0 deletions its/ruling/src/test/expected/javascript-S1154.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
'project:dojo-1.8.1/dijit/dijit-all.js.uncompressed.js':[
23138,
],
'project:dojo-1.8.1/dijit/form/ValidationTextBox.js.uncompressed.js':[
285,
],
}
5 changes: 5 additions & 0 deletions its/ruling/src/test/profile.xml
Original file line number Diff line number Diff line change
Expand Up @@ -609,5 +609,10 @@
<key>S3002</key>
<priority>INFO</priority> <!-- Unary operators "+" and "-" should not be used with objects -->
</rule>
<rule>
<repositoryKey>javascript</repositoryKey>
<key>S1154</key>
<priority>INFO</priority><!-- Results of operations on strings should not be ignored -->
</rule>
</rules>
</profile>
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public static List<Class> getChecks() {
NewOperatorMisuseCheck.class,
WebSQLDatabaseCheck.class,
PostMessageCheck.class,
UselessStringOperationCheck.class,
LocalStorageCheck.class,
UnaryPlusMinusWithObjectCheck.class,
BackboneChangedIsUsedCheck.class,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011 SonarSource and Eriks Nukis
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.javascript.checks;

import com.google.common.collect.ImmutableList;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.check.Priority;
import org.sonar.check.Rule;
import org.sonar.javascript.checks.utils.CheckUtils;
import org.sonar.plugins.javascript.api.symbols.Type;
import org.sonar.plugins.javascript.api.tree.Tree;
import org.sonar.plugins.javascript.api.tree.Tree.Kind;
import org.sonar.plugins.javascript.api.tree.expression.CallExpressionTree;
import org.sonar.plugins.javascript.api.tree.expression.ExpressionTree;
import org.sonar.plugins.javascript.api.tree.expression.MemberExpressionTree;
import org.sonar.plugins.javascript.api.tree.statement.ExpressionStatementTree;
import org.sonar.plugins.javascript.api.visitors.SubscriptionBaseTreeVisitor;
import org.sonar.squidbridge.annotations.ActivatedByDefault;
import org.sonar.squidbridge.annotations.SqaleConstantRemediation;
import org.sonar.squidbridge.annotations.SqaleSubCharacteristic;

import java.util.List;

@Rule(
key = "S1154",
name = "Results of operations on strings should not be ignored",
priority = Priority.BLOCKER,
tags = {Tags.BUG})
@ActivatedByDefault
@SqaleSubCharacteristic(RulesDefinition.SubCharacteristics.INSTRUCTION_RELIABILITY)
@SqaleConstantRemediation("20min")
public class UselessStringOperationCheck extends SubscriptionBaseTreeVisitor {

@Override
public List<Kind> nodesToVisit() {
return ImmutableList.of(Kind.EXPRESSION_STATEMENT);
}

@Override
public void visitNode(Tree tree) {
Tree expression = ((ExpressionStatementTree) tree).expression();
if (expression.is(Kind.CALL_EXPRESSION)) {
ExpressionTree callee = ((CallExpressionTree) expression).callee();
if (callee.is(Kind.DOT_MEMBER_EXPRESSION)) {
MemberExpressionTree memberExpression = (MemberExpressionTree) callee;
if (memberExpression.object().types().containsOnly(Type.Kind.STRING)) {
String variableName = CheckUtils.asString(memberExpression.object());
addIssue(tree, variableName + " is an immutable object; you must either store or return the result of the operation.");
}
}
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<p>Doing an operation on a string without using the result of the operation is useless and is certainly due to a misunderstanding. </p>
<h2>Noncompliant Code Example</h2>

<pre>
var str = "..."
str.toUpperCase(); // Noncompliant
</pre>
<h2>Compliant Solution</h2>

<pre>
var str = "..."
str = str.toUpperCase();
</pre>

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* SonarQube JavaScript Plugin
* Copyright (C) 2011 SonarSource and Eriks Nukis
* [email protected]
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this program; if not, write to the Free Software
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02
*/
package org.sonar.javascript.checks;

import org.junit.Test;
import org.sonar.plugins.javascript.api.tests.TreeCheckTest;
import org.sonar.squidbridge.checks.CheckMessagesVerifier;

public class UselessStringOperationCheckTest extends TreeCheckTest {

@Test
public void test() {
UselessStringOperationCheck check = new UselessStringOperationCheck();
CheckMessagesVerifier.verify(getIssues("src/test/resources/checks/UselessStringOperation.js", check))
.next().atLine(4).withMessage("str is an immutable object; you must either store or return the result of the operation.")
.next().atLine(5).withMessage("\"abc\" is an immutable object; you must either store or return the result of the operation.")
.next().atLine(6)
.noMore();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
var var1 = "abc".toUpperCase(); // OK
unknown.toUpperCase(); // OK
var str = "abcd";
str.toUpperCase(); // NOK
"abc".toUpperCase(); // NOK
str.substring(1,2); // NOK

var x = "abc";
x = something();
x.toUpperCase(); // OK
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void should_create_sonar_way_profile() {
assertThat(profile.getLanguage()).isEqualTo(JavaScriptLanguage.KEY);
assertThat(profile.getName()).isEqualTo(CheckList.SONAR_WAY_PROFILE);
assertThat(profile.getActiveRulesByRepository(CheckList.REPOSITORY_KEY))
.hasSize(77);
.hasSize(78);
assertThat(validation.hasErrors()).isFalse();
}

Expand Down

0 comments on commit d9b7fd6

Please sign in to comment.