Fix #75 FP on rule multiple-variable-declarations-same-scope within m…

…ixin calls when parameters are the same variables
racodond · Dec 6, 2017 · 87dde3c · 87dde3c
1 parent 158fcc4
commit 87dde3c
Show file tree

Hide file tree

Showing 8 changed files with 29 additions and 31 deletions.
diff --git a/css-checks/src/test/resources/checks/less/multipleVariableDeclarationsSameScope.less b/css-checks/src/test/resources/checks/less/multipleVariableDeclarationsSameScope.less
@@ -17,14 +17,16 @@
   border-radius: @radius;
 }
 
-.border-radius2(@radius, @radius) { /* Noncompliant ![sc=18;ec=24;secondary=+0]! !{Merge those multiple variable declarations within the same scope.}! */
+.border-radius2(@radius) {
   -webkit-border-radius: @radius;
   -moz-border-radius: @radius;
   border-radius: @radius;
+  @radius: 10;
 }
 
-.border-radius3(@radius: 3px, @radius: 4px) { /* Noncompliant ![sc=18;ec=24;secondary=+0]! !{Merge those multiple variable declarations within the same scope.}! */
-  -webkit-border-radius: @radius;
-  -moz-border-radius: @radius;
+@radius: 10;
+
+.myclass {
+  @radius: 10;
   border-radius: @radius;
 }
diff --git a/css-frontend/src/main/java/org/sonar/css/tree/symbol/SymbolVisitor.java b/css-frontend/src/main/java/org/sonar/css/tree/symbol/SymbolVisitor.java
@@ -73,24 +73,10 @@ public void visitLessMixinParameters(LessMixinParametersTree tree) {
     leaveScope();
   }
 
-  private void leaveScope() {
-    if (currentScope != null) {
-      currentScope = currentScope.outer();
-    }
-  }
-
-  private void enterScope(Tree tree) {
-    currentScope = treeScopeMap.get(tree);
-    if (currentScope == null) {
-      throw new IllegalStateException("No scope found for the tree");
-    }
-  }
-
   @Override
   public void visitLessVariable(LessVariableTree tree) {
     Usage.Kind usage;
-
-    if (tree.parent().is(Tree.Kind.LESS_VARIABLE_DECLARATION, Tree.Kind.LESS_MIXIN_PARAMETER)) {
+    if (tree.parent().is(Tree.Kind.LESS_VARIABLE_DECLARATION)) {
       usage = Usage.Kind.DECLARATION;
     } else {
       usage = Usage.Kind.READ;
@@ -108,4 +94,18 @@ private void initScopes() {
     currentScope = null;
   }
 
+
+  private void leaveScope() {
+    if (currentScope != null) {
+      currentScope = currentScope.outer();
+    }
+  }
+
+  private void enterScope(Tree tree) {
+    currentScope = treeScopeMap.get(tree);
+    if (currentScope == null) {
+      throw new IllegalStateException("No scope found for the tree");
+    }
+  }
+
 }
diff --git a/its/ruling/projects/custom/less/multipleVariableDeclarationsSameScope.less b/its/ruling/projects/custom/less/multipleVariableDeclarationsSameScope.less
@@ -17,14 +17,16 @@
   border-radius: @radius;
 }
 
-.border-radius2(@radius, @radius) { /* Noncompliant ![sc=18;ec=24;secondary=+0]! !{Merge those multiple variable declarations within the same scope.}! */
+.border-radius2(@radius) {
   -webkit-border-radius: @radius;
   -moz-border-radius: @radius;
   border-radius: @radius;
+  @radius: 10;
 }
 
-.border-radius3(@radius: 3px, @radius: 4px) { /* Noncompliant ![sc=18;ec=24;secondary=+0]! !{Merge those multiple variable declarations within the same scope.}! */
-  -webkit-border-radius: @radius;
-  -moz-border-radius: @radius;
+@radius: 10;
+
+.myclass {
+  @radius: 10;
   border-radius: @radius;
 }
diff --git a/its/ruling/tests/src/test/expected/less-experimental-property-usage.json b/its/ruling/tests/src/test/expected/less-experimental-property-usage.json
@@ -1886,8 +1886,6 @@
 16,
 21,
 22,
-27,
-28,
 ],
 'project:custom/less/obsoleteFunctions.less':[
 6,

diff --git a/its/ruling/tests/src/test/expected/less-line-length.json b/its/ruling/tests/src/test/expected/less-line-length.json
@@ -1045,8 +1045,6 @@
 'project:custom/less/multipleVariableDeclarationsSameScope.less':[
 1,
 5,
-20,
-26,
 ],
 'project:custom/less/nestedRulesets/nestedRulesets.less':[
 18,

diff --git a/its/ruling/tests/src/test/expected/less-multiple-variable-declarations-same-scope.json b/its/ruling/tests/src/test/expected/less-multiple-variable-declarations-same-scope.json
@@ -8,7 +8,5 @@
 'project:custom/less/multipleVariableDeclarationsSameScope.less':[
 1,
 5,
-20,
-26,
 ],
 }
diff --git a/its/ruling/tests/src/test/expected/less-prefer-single-line-comments.json b/its/ruling/tests/src/test/expected/less-prefer-single-line-comments.json
@@ -2116,8 +2116,6 @@
 'project:custom/less/multipleVariableDeclarationsSameScope.less':[
 1,
 5,
-20,
-26,
 ],
 'project:custom/less/nestedRulesets/nestedRulesets.less':[
 1,

diff --git a/its/ruling/tests/src/test/expected/less-variable-declaration-first.json b/its/ruling/tests/src/test/expected/less-variable-declaration-first.json
@@ -15,6 +15,8 @@
 8,
 9,
 12,
+24,
+27,
 ],
 'project:custom/less/variableDeclarationFirst.less':[
 19,
-Original file line number
+Diff line change
@@ Expand Up / @@ -1886,8 +1886,6 @@ @@
 ,
 ,
 ,
-,
-,
     ],
     'project:custom/less/obsoleteFunctions.less':[
 ,
@@ Expand Down @@