diff --git a/its/ruling/src/test/expected/javascript-S1154.json b/its/ruling/src/test/expected/javascript-S1154.json new file mode 100644 index 00000000000..102a11297de --- /dev/null +++ b/its/ruling/src/test/expected/javascript-S1154.json @@ -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, +], +} diff --git a/its/ruling/src/test/profile.xml b/its/ruling/src/test/profile.xml index 0944c3281f5..f9e0d14dd60 100644 --- a/its/ruling/src/test/profile.xml +++ b/its/ruling/src/test/profile.xml @@ -609,5 +609,10 @@ S3002 INFO + + javascript + S1154 + INFO + diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java index 904fd09ba33..c57541df3ed 100644 --- a/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/CheckList.java @@ -142,6 +142,7 @@ public static List getChecks() { NewOperatorMisuseCheck.class, WebSQLDatabaseCheck.class, PostMessageCheck.class, + UselessStringOperationCheck.class, LocalStorageCheck.class, UnaryPlusMinusWithObjectCheck.class, BackboneChangedIsUsedCheck.class, diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/UselessStringOperationCheck.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/UselessStringOperationCheck.java new file mode 100644 index 00000000000..582d8967c65 --- /dev/null +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/UselessStringOperationCheck.java @@ -0,0 +1,71 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011 SonarSource and Eriks Nukis + * sonarqube@googlegroups.com + * + * 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 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."); + } + } + } + } + +} diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1154.html b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1154.html new file mode 100644 index 00000000000..0d03f3fac95 --- /dev/null +++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/S1154.html @@ -0,0 +1,14 @@ +

Doing an operation on a string without using the result of the operation is useless and is certainly due to a misunderstanding.

+

Noncompliant Code Example

+ +
+var str = "..."
+str.toUpperCase(); // Noncompliant
+
+

Compliant Solution

+ +
+var str = "..."
+str = str.toUpperCase();
+
+ diff --git a/javascript-checks/src/test/java/org/sonar/javascript/checks/UselessStringOperationCheckTest.java b/javascript-checks/src/test/java/org/sonar/javascript/checks/UselessStringOperationCheckTest.java new file mode 100644 index 00000000000..7af84bd4d06 --- /dev/null +++ b/javascript-checks/src/test/java/org/sonar/javascript/checks/UselessStringOperationCheckTest.java @@ -0,0 +1,38 @@ +/* + * SonarQube JavaScript Plugin + * Copyright (C) 2011 SonarSource and Eriks Nukis + * sonarqube@googlegroups.com + * + * 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(); + } + +} diff --git a/javascript-checks/src/test/resources/checks/UselessStringOperation.js b/javascript-checks/src/test/resources/checks/UselessStringOperation.js new file mode 100644 index 00000000000..8caac0b7183 --- /dev/null +++ b/javascript-checks/src/test/resources/checks/UselessStringOperation.js @@ -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 diff --git a/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptProfileTest.java b/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptProfileTest.java index ddb16817cc0..fb949b74108 100644 --- a/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptProfileTest.java +++ b/sonar-javascript-plugin/src/test/java/org/sonar/plugins/javascript/JavaScriptProfileTest.java @@ -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(); }