diff --git a/javascript-checks/src/main/java/org/sonar/javascript/checks/DuplicatePropertyNameCheck.java b/javascript-checks/src/main/java/org/sonar/javascript/checks/DuplicatePropertyNameCheck.java index 904cd6043c4..c82b8d5c53b 100644 --- a/javascript-checks/src/main/java/org/sonar/javascript/checks/DuplicatePropertyNameCheck.java +++ b/javascript-checks/src/main/java/org/sonar/javascript/checks/DuplicatePropertyNameCheck.java @@ -19,11 +19,16 @@ */ package org.sonar.javascript.checks; -import java.util.HashMap; -import java.util.Map; +import com.google.common.collect.LinkedListMultimap; +import com.google.common.collect.ListMultimap; +import java.util.List; +import javax.annotation.Nullable; import org.sonar.check.Rule; import org.sonar.plugins.javascript.api.tree.Tree; import org.sonar.plugins.javascript.api.tree.Tree.Kind; +import org.sonar.plugins.javascript.api.tree.declaration.FieldDeclarationTree; +import org.sonar.plugins.javascript.api.tree.declaration.MethodDeclarationTree; +import org.sonar.plugins.javascript.api.tree.expression.ClassTree; import org.sonar.plugins.javascript.api.tree.expression.IdentifierTree; import org.sonar.plugins.javascript.api.tree.expression.LiteralTree; import org.sonar.plugins.javascript.api.tree.expression.ObjectLiteralTree; @@ -37,45 +42,90 @@ public class DuplicatePropertyNameCheck extends DoubleDispatchVisitorCheck { @Override public void visitObjectLiteral(ObjectLiteralTree tree) { - Map keys = new HashMap<>(); + checkProperties(tree.properties()); + super.visitObjectLiteral(tree); + } - for (Tree property : tree.properties()) { - if (property.is(Tree.Kind.PAIR_PROPERTY)) { - visitPairProperty(keys, (PairPropertyTree) property); - } + @Override + public void visitClass(ClassTree tree) { + checkProperties(tree.elements()); + super.visitClass(tree); + } + + private void checkProperties(List properties) { + ListMultimap keys = LinkedListMultimap.create(); - if (property.is(Kind.IDENTIFIER_REFERENCE)) { - IdentifierTree identifier = (IdentifierTree) property; - addKey(keys, identifier.name(), (IdentifierTree)property); + for (Tree property : properties) { + Tree propertyNameTree = getPropertyNameTree(property); + if (propertyNameTree != null) { + String propertyName = getPropertyName(propertyNameTree); + if (propertyName != null) { + keys.put(EscapeUtils.unescape(propertyName), property); + } } } - super.visitObjectLiteral(tree); + + checkKeys(keys); } - private void visitPairProperty(Map keys, PairPropertyTree pairProperty) { - Tree key = pairProperty.key(); - if (key.is(Tree.Kind.STRING_LITERAL)) { - String value = ((LiteralTree) key).value(); - value = value.substring(1, value.length() - 1); - addKey(keys, value, pairProperty.key()); + private void checkKeys(ListMultimap keys) { + for (String key : keys.keySet()) { + List properties = keys.get(key); + if (properties.size() > 1 && !getterSetter(properties)) { + Tree duplicatedProperty = getPropertyNameTree(properties.remove(0)); + + for (Tree property : properties) { + Tree propertyKey = getPropertyNameTree(property); + addIssue(propertyKey, String.format(MESSAGE, getPropertyName(propertyKey))).secondary(duplicatedProperty); + } + + } } + } - if (key.is(Kind.IDENTIFIER_NAME)) { - addKey(keys, ((IdentifierTree) key).name(), pairProperty.key()); + private static boolean getterSetter(List value) { + if (value.size() == 2) { + return (value.get(0).is(Kind.GET_METHOD) && value.get(1).is(Kind.SET_METHOD)) + || (value.get(1).is(Kind.GET_METHOD) && value.get(0).is(Kind.SET_METHOD)); } - if (key.is(Tree.Kind.NUMERIC_LITERAL)) { - addKey(keys, ((LiteralTree) key).value(), pairProperty.key()); + return false; + } + + @Nullable + private static Tree getPropertyNameTree(Tree property) { + if (property instanceof MethodDeclarationTree) { + return ((MethodDeclarationTree) property).name(); + + } else if (property.is(Kind.FIELD)) { + return ((FieldDeclarationTree) property).propertyName(); + + } else if (property.is(Kind.PAIR_PROPERTY)) { + return ((PairPropertyTree) property).key(); + + } else if (property.is(Kind.IDENTIFIER_REFERENCE)) { + return property; } + + return null; } - private void addKey(Map keys, String key, Tree keyTree) { - Tree duplicated = keys.get(EscapeUtils.unescape(key)); - if (duplicated != null) { - addIssue(keyTree, String.format(MESSAGE, key)).secondary(duplicated); - } else { - keys.put(key, keyTree); + @Nullable + private static String getPropertyName(Tree propertyKey) { + if (propertyKey.is(Tree.Kind.STRING_LITERAL)) { + String value = ((LiteralTree) propertyKey).value(); + return value.substring(1, value.length() - 1); } + + if (propertyKey.is(Kind.IDENTIFIER_NAME, Kind.IDENTIFIER_REFERENCE)) { + return ((IdentifierTree) propertyKey).name(); + } + + if (propertyKey.is(Tree.Kind.NUMERIC_LITERAL)) { + return ((LiteralTree) propertyKey).value(); + } + + return null; } } diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.html b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.html index d8fa29277a8..fac87c1574d 100644 --- a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.html +++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.html @@ -1,7 +1,7 @@ -

JavaScript allows duplicate property names in object literals, but only the last instance of a duplicated property determines the actual value that -will be used for it. Therefore, changing values of other occurrences of a duplicated property will have no effect and may cause misunderstandings and -bugs.

-

What's more, using duplicate property names will generate an error in JavaScript strict mode code.

+

JavaScript allows duplicate property names in classes and object literals, but only the last instance of a duplicated name determines the actual +value that will be used for it. Therefore, changing values of other occurrences of a duplicated name will have no effect and may cause +misunderstandings and bugs.

+

What's more, using duplicate names will generate an error in JavaScript strict mode code.

Noncompliant Code Example

 var data = {
diff --git a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.json b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.json
index 18a32d10927..d37f2213e3a 100644
--- a/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.json
+++ b/javascript-checks/src/main/resources/org/sonar/l10n/javascript/rules/javascript/DuplicatePropertyName.json
@@ -1,5 +1,5 @@
 {
-  "title": "Property names should not be duplicated within an object literal",
+  "title": "Property names should not be duplicated within a class or object literal ",
   "type": "BUG",
   "status": "ready",
   "remediation": {
diff --git a/javascript-checks/src/test/resources/checks/duplicatePropertyName.js b/javascript-checks/src/test/resources/checks/duplicatePropertyName.js
index 168b897bc88..9bdcd47a5a3 100644
--- a/javascript-checks/src/test/resources/checks/duplicatePropertyName.js
+++ b/javascript-checks/src/test/resources/checks/duplicatePropertyName.js
@@ -15,7 +15,37 @@ var data = {
   key,                 // Noncompliant
 //^^^
 
-  get key() {        // OK - accessor
+  get key() {        // Noncompliant
     return this.key;
-  }
+  },
+
+  set key(p) {      // Noncompliant
+  },
+
+  key() {},  // Noncompliant
+  ["computedProperty"] : 1
+}
+
+class A {
+  prop;
+  prop1;
+  ;
+  prop = 1;        // Noncompliant
+  static prop = 1; // Noncompliant
+  prop() {}        // Noncompliant
+  * prop() {}      // Noncompliant
+  set prop(p) {}   // Noncompliant
+  get prop() {}    // Noncompliant
+
+  set prop2(p) {}
+  prop2;           // Noncompliant
+
+  get prop3() {}
+  prop3;            // Noncompliant
+
+  get prop4() {}
+  set prop4(p) {}    // OK
+
+  set prop5(p) {}    // OK
+  get prop5() {}
 }
diff --git a/javascript-frontend/src/main/java/org/sonar/plugins/javascript/api/tree/expression/ClassTree.java b/javascript-frontend/src/main/java/org/sonar/plugins/javascript/api/tree/expression/ClassTree.java
index d1b022a2f89..b85620fa5fb 100644
--- a/javascript-frontend/src/main/java/org/sonar/plugins/javascript/api/tree/expression/ClassTree.java
+++ b/javascript-frontend/src/main/java/org/sonar/plugins/javascript/api/tree/expression/ClassTree.java
@@ -55,7 +55,7 @@ public interface ClassTree extends ExpressionTree, StatementTree {
 
   SyntaxToken openCurlyBraceToken();
 
-  // elements can be either method declarations or semi-colons
+  // elements can be either method declarations, class field declarations or semi-colons
   List elements();
 
   Iterable methods();
diff --git a/sonar-javascript-plugin/pom.xml b/sonar-javascript-plugin/pom.xml
index aaa6676a53d..629562dbf5e 100644
--- a/sonar-javascript-plugin/pom.xml
+++ b/sonar-javascript-plugin/pom.xml
@@ -109,8 +109,8 @@
             
               
                 
-                  2800000
-                  2900000
+                  2900000
+                  3000000
                   
                     ${project.build.directory}/${project.build.finalName}.jar