Remove SeparatorParameter to avoid breaking SonarSecurity (SonarSource#634)

pynicolas · web-flow · commit da04c8a20975 · 2020-03-13T11:27:03.000+01:00
diff --git a/python-checks/src/main/java/org/sonar/python/checks/InfiniteRecursionCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InfiniteRecursionCheck.java
@@ -270,7 +270,7 @@ private static Symbol findSelfParameterSymbol(FunctionDef functionDef) {
         return null;
       }
       Name firstParameterName = params.get(0).name();
-      return firstParameterName.symbol();
+      return firstParameterName != null ? firstParameterName.symbol() : null;
     }
 
     @CheckForNull
diff --git a/python-checks/src/main/java/org/sonar/python/checks/MethodShouldBeStaticCheck.java b/python-checks/src/main/java/org/sonar/python/checks/MethodShouldBeStaticCheck.java
@@ -86,12 +86,13 @@ private static boolean isUsingSelfArg(FunctionDef funcDef) {
     }
     if (params.get(0).is(Tree.Kind.TUPLE_PARAMETER)) {
       return false;
-    } else if (params.get(0).is(Tree.Kind.SEPARATOR_PARAMETER)) {
-      // star argument should not raise issue
-      return true;
     }
     Parameter first = (Parameter) params.get(0);
     Name paramName = first.name();
+    if (paramName == null) {
+      // star argument should not raise issue
+      return true;
+    }
     SelfVisitor visitor = new SelfVisitor(paramName.name());
     funcDef.body().accept(visitor);
     return visitor.isUsingSelfArg;
diff --git a/python-checks/src/main/java/org/sonar/python/checks/NoReRaiseInExitCheck.java b/python-checks/src/main/java/org/sonar/python/checks/NoReRaiseInExitCheck.java
@@ -30,6 +30,7 @@
 import org.sonar.plugins.python.api.tree.Expression;
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.HasSymbol;
+import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.Parameter;
 import org.sonar.plugins.python.api.tree.ParameterList;
 import org.sonar.plugins.python.api.tree.RaiseStatement;
@@ -88,7 +89,11 @@ private static Symbol extractPackedParameter(ParameterList parameterList) {
       return null;
     }
 
-    return parameter.name().symbol();
+    Name name = parameter.name();
+    if (name == null) {
+      return null;
+    }
+    return name.symbol();
   }
 
   private static Symbol extractCaughtExceptionParameter(ParameterList parameterList) {
@@ -100,7 +105,11 @@ private static Symbol extractCaughtExceptionParameter(ParameterList parameterLis
     }
 
     Parameter parameter = regularParams.get(2);
-    return parameter.name().symbol();
+    Name name = parameter.name();
+    if (name == null) {
+      return null;
+    }
+    return name.symbol();
   }
 
   @Override
diff --git a/python-checks/src/main/java/org/sonar/python/checks/SpecialMethodParamListCheck.java b/python-checks/src/main/java/org/sonar/python/checks/SpecialMethodParamListCheck.java
@@ -132,7 +132,7 @@ public void initialize(Context context) {
       }
 
       // Check if the method was declared with packed arguments.
-      if (hasPackedOrKeywordParameter(parameterList) || parameterList.stream().anyMatch(p -> p.is(Tree.Kind.SEPARATOR_PARAMETER))) {
+      if (hasPackedOrKeywordParameter(parameterList)) {
         return;
       }
 
diff --git a/python-checks/src/main/java/org/sonar/python/checks/hotspots/HardCodedCredentialsCheck.java b/python-checks/src/main/java/org/sonar/python/checks/hotspots/HardCodedCredentialsCheck.java
@@ -142,6 +142,9 @@ private void checkKeyValuePair(KeyValuePair keyValuePair, SubscriptionContext ct
   private void handleParameterList(ParameterList parameterList, SubscriptionContext ctx) {
     for (Parameter parameter : parameterList.nonTuple()) {
       Name parameterName = parameter.name();
+      if (parameterName == null) {
+        continue;
+      }
       Expression defaultValue = parameter.defaultValue();
       String matchedCredential = matchedCredential(parameterName.name(), variablePatterns());
       if (matchedCredential != null && defaultValue != null && isSuspiciousStringLiteral(defaultValue)) {
diff --git a/python-checks/src/test/resources/checks/infiniteRecursion.py b/python-checks/src/test/resources/checks/infiniteRecursion.py
@@ -341,3 +341,6 @@ async def asyn5(i): # OK, await is not on "asyn5" call
 
 async def asyn6(i): # Noncompliant
   await some_call(await asyn6(i-1), 42)
+
+class A:
+  def func(*, x, y): ...
diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/BaseTreeVisitor.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/BaseTreeVisitor.java
@@ -299,11 +299,6 @@ public void visitParameter(Parameter tree) {
     scan(tree.defaultValue());
   }
 
-  @Override
-  public void visitSeparatorParameter(SeparatorParameter parameter) {
-    // noop
-  }
-
   @Override
   public void visitTypeAnnotation(TypeAnnotation tree) {
     scan(tree.expression());
diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Parameter.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Parameter.java
@@ -43,6 +43,7 @@ public interface Parameter extends AnyParameter {
   @CheckForNull
   Token starToken();
 
+  @CheckForNull
   Name name();
 
   @CheckForNull
diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/SeparatorParameter.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/SeparatorParameter.java
diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Tree.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/Tree.java
@@ -137,7 +137,6 @@ enum Kind {
     TRY_STMT(TryStatement.class),
 
     PARAMETER(Parameter.class),
-    SEPARATOR_PARAMETER(SeparatorParameter.class),
     TUPLE_PARAMETER(TupleParameter.class),
 
     VARIABLE_TYPE_ANNOTATION(TypeAnnotation.class),
diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/TreeVisitor.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/tree/TreeVisitor.java
@@ -111,8 +111,6 @@ public interface TreeVisitor {
 
   void visitParameter(Parameter parameter);
 
-  void visitSeparatorParameter(SeparatorParameter parameter);
-
   void visitTypeAnnotation(TypeAnnotation typeAnnotation);
 
   void visitNumericLiteral(NumericLiteral numericLiteral);
diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/FunctionSymbolImpl.java b/python-frontend/src/main/java/org/sonar/python/semantic/FunctionSymbolImpl.java
@@ -32,6 +32,7 @@
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.ParameterList;
+import org.sonar.plugins.python.api.tree.Token;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.plugins.python.api.types.InferredType;
 import org.sonar.python.TokenLocation;
@@ -112,12 +113,13 @@ private void createParameterNames(List<AnyParameter> parameterTrees) {
       if (anyParameter.is(Tree.Kind.PARAMETER)) {
         org.sonar.plugins.python.api.tree.Parameter parameter = (org.sonar.plugins.python.api.tree.Parameter) anyParameter;
         Name parameterName = parameter.name();
-        this.parameters.add(new ParameterImpl(parameterName.name(), parameter.defaultValue() != null, keywordOnly));
-        if (parameter.starToken() != null) {
-          hasVariadicParameter = true;
-        }
-      } else if (anyParameter.is(Tree.Kind.SEPARATOR_PARAMETER)) {
-        if (anyParameter.firstToken().value().equals("*")) {
+        Token starToken = parameter.starToken();
+        if (parameterName != null) {
+          this.parameters.add(new ParameterImpl(parameterName.name(), parameter.defaultValue() != null, keywordOnly));
+          if (starToken != null) {
+            hasVariadicParameter = true;
+          }
+        } else if (starToken != null && "*".equals(starToken.value())) {
           keywordOnly = true;
         }
       } else {
diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java b/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java
@@ -93,6 +93,9 @@ void createSymbolsFromWildcardImport(Set<Symbol> importedSymbols, Map<String, Sy
 
   void createSelfParameter(Parameter parameter) {
     Name nameTree = parameter.name();
+    if (nameTree == null) {
+      return;
+    }
     String symbolName = nameTree.name();
     SymbolImpl symbol = new SelfSymbolImpl(symbolName, parent);
     symbols.add(symbol);
diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java b/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java
@@ -29,6 +29,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
@@ -371,6 +372,7 @@ private void createParameters(FunctionLike function) {
         .stream()
         .skip(hasSelf ? 1 : 0)
         .map(Parameter::name)
+        .filter(Objects::nonNull)
         .forEach(param -> addBindingUsage(param, Usage.Kind.PARAMETER));
 
       parameterList.all().stream()
diff --git a/python-frontend/src/main/java/org/sonar/python/tree/ParameterImpl.java b/python-frontend/src/main/java/org/sonar/python/tree/ParameterImpl.java
@@ -50,12 +50,25 @@ public ParameterImpl(@Nullable Token starToken, Name name, @Nullable TypeAnnotat
     this.defaultValue = defaultValue;
   }
 
+  /**
+   * constructor for star parameter syntax.
+   * def fun(arg1, *, arg2)
+   */
+  public ParameterImpl(Token starToken) {
+    this.starToken = starToken;
+    this.name = null;
+    this.annotation = null;
+    this.equalToken = null;
+    this.defaultValue = null;
+  }
+
   @CheckForNull
   @Override
   public Token starToken() {
     return starToken;
   }
 
+  @CheckForNull
   @Override
   public Name name() {
     return name;
diff --git a/python-frontend/src/main/java/org/sonar/python/tree/PythonTreeMaker.java b/python-frontend/src/main/java/org/sonar/python/tree/PythonTreeMaker.java
@@ -1218,11 +1218,11 @@ public LambdaExpression lambdaExpression(AstNode astNode) {
 
   private AnyParameter parameter(AstNode parameter) {
     if (parameter.is(PythonPunctuator.DIV)) {
-      return new SeparatorParameterImpl(toPyToken(parameter.getToken()));
+      return new ParameterImpl(toPyToken(parameter.getToken()));
     }
     if (parameter.is(PythonPunctuator.MUL)) {
       if (parameter.getNextSibling() == null || parameter.getNextSibling().is(PythonPunctuator.COMMA)) {
-        return new SeparatorParameterImpl(toPyToken(parameter.getToken()));
+        return new ParameterImpl(toPyToken(parameter.getToken()));
       }
       return null;
     }
diff --git a/python-frontend/src/main/java/org/sonar/python/tree/SeparatorParameterImpl.java b/python-frontend/src/main/java/org/sonar/python/tree/SeparatorParameterImpl.java
diff --git a/python-frontend/src/test/java/org/sonar/python/tree/PythonTreeMakerTest.java b/python-frontend/src/test/java/org/sonar/python/tree/PythonTreeMakerTest.java
@@ -89,7 +89,6 @@
 import org.sonar.plugins.python.api.tree.RegularArgument;
 import org.sonar.plugins.python.api.tree.ReprExpression;
 import org.sonar.plugins.python.api.tree.ReturnStatement;
-import org.sonar.plugins.python.api.tree.SeparatorParameter;
 import org.sonar.plugins.python.api.tree.SetLiteral;
 import org.sonar.plugins.python.api.tree.SliceExpression;
 import org.sonar.plugins.python.api.tree.SliceItem;
@@ -846,11 +845,11 @@ public void funcdef_statement() {
     functionDef = parse("def __call__(self, *, manager):\n  pass", treeMaker::funcDefStatement);
     assertThat(functionDef.parameters().all()).hasSize(3);
     functionDef = parse("def __call__(*):\n  pass", treeMaker::funcDefStatement);
-    assertThat(functionDef.parameters().all()).extracting(Tree::getKind).containsExactly(Kind.SEPARATOR_PARAMETER);
+    assertThat(functionDef.parameters().all()).extracting(Tree::getKind).containsExactly(Kind.PARAMETER);
 
     functionDef = parse("def f(a, /): pass", treeMaker::funcDefStatement);
-    assertThat(functionDef.parameters().all()).extracting(Tree::getKind).containsExactly(Kind.PARAMETER, Kind.SEPARATOR_PARAMETER);
-    assertThat(((SeparatorParameter) functionDef.parameters().all().get(1)).starOrSlashToken().value()).isEqualTo("/");
+    assertThat(functionDef.parameters().all()).extracting(Tree::getKind).containsExactly(Kind.PARAMETER, Kind.PARAMETER);
+    assertThat(((Parameter) functionDef.parameters().all().get(1)).starToken().value()).isEqualTo("/");
 
     assertThat(funcDef("def func(): ...").parameters()).isNull();
     assertThat(funcDef("def func(a): ...").parameters().all()).hasSize(1);

Original file line number	Diff line number	Diff line change
`@@ -270,7 +270,7 @@ private static Symbol findSelfParameterSymbol(FunctionDef functionDef) {`
`270`	`270`	`return null;`
`271`	`271`	`}`
`272`	`272`	`Name firstParameterName = params.get(0).name();`
`273`		`- return firstParameterName.symbol();`
	`273`	`+ return firstParameterName != null ? firstParameterName.symbol() : null;`
`274`	`274`	`}`
`275`	`275`
`276`	`276`	`@CheckForNull`
Original file line number	Diff line number	Diff line change
`@@ -132,7 +132,7 @@ public void initialize(Context context) {`
`132`	`132`	`}`
`133`	`133`
`134`	`134`	`// Check if the method was declared with packed arguments.`
`135`		`- if (hasPackedOrKeywordParameter(parameterList) \|\| parameterList.stream().anyMatch(p -> p.is(Tree.Kind.SEPARATOR_PARAMETER))) {`
	`135`	`+ if (hasPackedOrKeywordParameter(parameterList)) {`
`136`	`136`	`return;`
`137`	`137`	`}`
`138`	`138`