Cleanup of Tree API

its/ruling/src/test/resources/expected/python-S108.json

@@ -57,6 +57,10 @@
 112,
 144,
 ],
+'project:numpy-1.16.4/numpy/core/einsumfunc.py':[
+832,
+836,
+],
 'project:numpy-1.16.4/numpy/core/records.py':[
 179,
 ],
@@ -124,6 +128,9 @@
 'project:twisted-12.1.0/twisted/internet/_threadedselect.py':[
 281,
 ],
+'project:twisted-12.1.0/twisted/internet/tcp.py':[
+562,
+],
 'project:twisted-12.1.0/twisted/mail/test/test_pop3.py':[
 72,
 ],

python-checks/src/main/java/org/sonar/python/checks/EmptyNestedBlockCheck.java

@@ -20,14 +20,14 @@
 package org.sonar.python.checks;
 
 import java.util.List;
-import java.util.stream.Collectors;
 import org.sonar.check.Rule;
 import org.sonar.python.PythonSubscriptionCheck;
-import org.sonar.python.api.tree.Statement;
+import org.sonar.python.api.PythonTokenType;
 import org.sonar.python.api.tree.StatementList;
 import org.sonar.python.api.tree.Token;
 import org.sonar.python.api.tree.Tree;
 import org.sonar.python.api.tree.Tree.Kind;
+import org.sonar.python.tree.TreeUtils;
 
 @Rule(key = EmptyNestedBlockCheck.CHECK_KEY)
 public class EmptyNestedBlockCheck extends PythonSubscriptionCheck {
@@ -38,17 +38,20 @@ public class EmptyNestedBlockCheck extends PythonSubscriptionCheck {
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Kind.STATEMENT_LIST, ctx -> {
       StatementList statementListTree = (StatementList) ctx.syntaxNode();
-      List<Statement> nonPassStatements = statementListTree.statements().stream()
-        .filter(stmt -> !stmt.is(Kind.PASS_STMT))
-        .collect(Collectors.toList());
-      if (!nonPassStatements.isEmpty()) {
+      if (statementListTree.statements().stream().anyMatch(stmt -> !stmt.is(Kind.PASS_STMT))) {
         return;
       }
       Tree parent = statementListTree.parent();
       if (parent.is(Kind.FUNCDEF) || parent.is(Kind.CLASSDEF) || parent.is(Kind.EXCEPT_CLAUSE)) {
         return;
       }
-      if (!containsComment(statementListTree.tokens())) {
+      List<Token> parentTokens = TreeUtils.tokens(statementListTree.parent());
+      int from = parentTokens.stream().filter(t -> t.type() == PythonTokenType.NEWLINE).findFirst()
+        .map(parentTokens::indexOf)
+        .orElseThrow(() -> new IllegalStateException(String.format("No newline token in parent of statement list at line %s", statementListTree.firstToken().line())));
+      // sublist call is excluding last index and token following last token of statement list (dedent) should be included in the comment verification.
+      int to = parentTokens.indexOf(statementListTree.lastToken()) + 2;
+      if (!containsComment(parentTokens.subList(from, to))) {
         if (statementListTree.statements().isEmpty()) {
           ctx.addIssue(statementListTree.firstToken(), MESSAGE);
         } else {
@@ -59,11 +62,6 @@ public void initialize(Context context) {
   }
 
   private static boolean containsComment(List<Token> tokens) {
-    for (Token token : tokens) {
-      if (!token.trivia().isEmpty()) {
-        return true;
-      }
-    }
-    return false;
+    return tokens.stream().anyMatch(t -> !t.trivia().isEmpty());
   }
 }

python-checks/src/main/java/org/sonar/python/checks/LoopExecutingAtMostOnceCheck.java

@@ -87,7 +87,7 @@ private static void checkLoop(CfgBranchingBlock loopBlock, SubscriptionContext c
           .forEach(workList::push);
       }
     }
-    if (loop.descendants(Kind.TRY_STMT).findFirst().isPresent()) {
+    if (TreeUtils.hasDescendant(loop, t -> t.is(Kind.TRY_STMT))) {
       return;
     }
     PreciseIssue issue = ctx.addIssue(loop.firstToken(), "Refactor this loop to do more than one iteration.");

python-checks/src/main/java/org/sonar/python/checks/TooManyLinesInFileCheck.java

@@ -23,9 +23,7 @@
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
 import org.sonar.python.PythonSubscriptionCheck;
-import org.sonar.python.api.tree.FileInput;
 import org.sonar.python.api.tree.Tree;
-import org.sonar.python.tree.BaseTreeVisitor;
 
 @Rule(key = TooManyLinesInFileCheck.CHECK_KEY)
 public class TooManyLinesInFileCheck extends PythonSubscriptionCheck {
@@ -42,24 +40,11 @@ public class TooManyLinesInFileCheck extends PythonSubscriptionCheck {
   @Override
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> {
-      FileVisitor visitor = new FileVisitor();
-      ctx.syntaxNode().accept(visitor);
-      if (visitor.numberOfLines > maximum) {
-        ctx.addFileIssue(MessageFormat.format(MESSAGE, ctx.pythonFile().fileName(), visitor.numberOfLines, maximum));
+      int line = ctx.syntaxNode().lastToken().line();
+      if (line > maximum) {
+        ctx.addFileIssue(MessageFormat.format(MESSAGE, ctx.pythonFile().fileName(), line, maximum));
       }
     });
   }
-
-  private static class FileVisitor extends BaseTreeVisitor {
-
-    private int numberOfLines = 0;
-
-    @Override
-    public void visitFileInput(FileInput fileInput) {
-      if (fileInput.statements() != null) {
-        numberOfLines = fileInput.statements().tokens().get(fileInput.statements().tokens().size() - 1).line();
-      }
-    }
-  }
 }
 

python-checks/src/main/java/org/sonar/python/checks/UnusedLocalVariableCheck.java

@@ -19,33 +19,30 @@
  */
 package org.sonar.python.checks;
 
-import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
 import org.sonar.check.Rule;
 import org.sonar.python.PythonSubscriptionCheck;
 import org.sonar.python.api.tree.CallExpression;
+import org.sonar.python.api.tree.Expression;
 import org.sonar.python.api.tree.ExpressionList;
 import org.sonar.python.api.tree.ForStatement;
 import org.sonar.python.api.tree.FunctionDef;
 import org.sonar.python.api.tree.Name;
 import org.sonar.python.api.tree.StringElement;
-import org.sonar.python.api.tree.StringLiteral;
 import org.sonar.python.api.tree.Tree;
 import org.sonar.python.api.tree.Tree.Kind;
 import org.sonar.python.semantic.Symbol;
 import org.sonar.python.semantic.Usage;
+import org.sonar.python.tree.BaseTreeVisitor;
 import org.sonar.python.tree.TreeUtils;
 
 @Rule(key = "S1481")
 public class UnusedLocalVariableCheck extends PythonSubscriptionCheck {
 
-  private static final Pattern INTERPOLATION_PATTERN = Pattern.compile("\\{(.*?)\\}");
-
   private static final String MESSAGE = "Remove the unused local variable \"%s\".";
 
   @Override
@@ -81,29 +78,39 @@ private static boolean isTupleDeclaration(Tree tree) {
   }
 
   private static boolean isCallingLocalsFunction(FunctionDef functionTree) {
-    return functionTree
-      .descendants(Kind.CALL_EXPR)
-      .map(CallExpression.class::cast)
-      .map(CallExpression::callee)
-      .anyMatch(callee -> callee.is(Kind.NAME) && "locals".equals(((Name) callee).name()));
+    return TreeUtils.hasDescendant(functionTree, t -> t.is(Kind.CALL_EXPR) && calleeHasNameLocals(((CallExpression) t)));
+  }
+
+  private static boolean calleeHasNameLocals(CallExpression callExpression) {
+    Expression callee = callExpression.callee();
+    return callee.is(Kind.NAME) && "locals".equals(((Name) callee).name());
   }
 
   private static Set<String> extractStringInterpolationIdentifiers(FunctionDef functionTree) {
-    return functionTree.descendants(Kind.STRING_LITERAL)
-      .map(StringLiteral.class::cast)
-      .flatMap(str -> str.stringElements().stream())
-      .filter(str -> str.prefix().equalsIgnoreCase("f"))
-      .map(StringElement::trimmedQuotesValue)
-      .flatMap(UnusedLocalVariableCheck::extractInterpolations)
-      .collect(Collectors.toSet());
+    StringInterpolationVisitor visitor = new StringInterpolationVisitor();
+    functionTree.accept(visitor);
+    return visitor.stringInterpolations;
   }
 
-  private static Stream<String> extractInterpolations(String str) {
-    Matcher matcher = INTERPOLATION_PATTERN.matcher(str);
-    List<String> identifiers = new ArrayList<>();
-    while (matcher.find()) {
-      identifiers.add(matcher.group(1));
+  private static class StringInterpolationVisitor extends BaseTreeVisitor {
+    private static final Pattern INTERPOLATION_PATTERN = Pattern.compile("\\{(.*?)\\}");
+
+    Set<String> stringInterpolations = new HashSet<>();
+    @Override
+    public void visitStringElement(StringElement tree) {
+      if(tree.prefix().equalsIgnoreCase("f")) {
+        stringInterpolations.addAll(extractInterpolations(tree.trimmedQuotesValue()));
+      }
     }
-    return identifiers.stream();
+
+    private static Set<String> extractInterpolations(String str) {
+      Matcher matcher = INTERPOLATION_PATTERN.matcher(str);
+      Set<String> identifiers = new HashSet<>();
+      while (matcher.find()) {
+        identifiers.add(matcher.group(1));
+      }
+      return identifiers;
+    }
+
   }
 }

python-checks/src/main/java/org/sonar/python/checks/hotspots/SQLQueriesCheck.java

@@ -20,12 +20,13 @@
 package org.sonar.python.checks.hotspots;
 
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.sonar.check.Rule;
 import org.sonar.python.SubscriptionContext;
+import org.sonar.python.api.tree.AliasedName;
 import org.sonar.python.api.tree.Argument;
 import org.sonar.python.api.tree.BinaryExpression;
 import org.sonar.python.api.tree.CallExpression;
@@ -66,20 +67,30 @@ private void visitFile(SubscriptionContext ctx) {
     isUsingDjangoModel = false;
     isUsingDjangoDBConnection = false;
     FileInput tree = (FileInput) ctx.syntaxNode();
-    List<Symbol> symbols = tree.descendants()
-      .filter(node -> node.is(Tree.Kind.IMPORT_FROM) || node.is(Tree.Kind.IMPORT_NAME))
-      .flatMap(node -> node.descendants(Tree.Kind.NAME))
-      .map(node -> ((Name) node).symbol())
+    SymbolsFromImport visitor = new SymbolsFromImport();
+    tree.accept(visitor);
+    visitor.symbols.stream()
+      .filter(Objects::nonNull)
+      .map(Symbol::fullyQualifiedName)
       .filter(Objects::nonNull)
-      .collect(Collectors.toList());
-    for (Symbol symbol : symbols) {
-      String qualifiedName = symbol.fullyQualifiedName() != null ? symbol.fullyQualifiedName() : "";
+      .forEach(qualifiedName -> {
       if (qualifiedName.contains("django.db.models")) {
         isUsingDjangoModel = true;
       }
       if (qualifiedName.contains("django.db.connection")) {
         isUsingDjangoDBConnection = true;
       }
+    });
+  }
+
+  private static class SymbolsFromImport extends BaseTreeVisitor {
+
+    private Set<Symbol> symbols = new HashSet<>();
+
+    @Override
+    public void visitAliasedName(AliasedName aliasedName) {
+      List<Name> names = aliasedName.dottedName().names();
+      symbols.add(names.get(names.size() - 1).symbol());
     }
   }
 

python-checks/src/main/java/org/sonar/python/checks/hotspots/StandardInputCheck.java

@@ -29,7 +29,6 @@
 import org.sonar.python.api.tree.Tree;
 import org.sonar.python.checks.AbstractCallExpressionCheck;
 import org.sonar.python.semantic.Symbol;
-import org.sonar.python.tree.TreeUtils;
 
 @Rule(key = StandardInputCheck.CHECK_KEY)
 public class StandardInputCheck extends AbstractCallExpressionCheck {
@@ -53,12 +52,12 @@ public void initialize(Context context) {
       }
     });
     context.registerSyntaxNodeConsumer(Tree.Kind.NAME, ctx -> {
-      Name node = (Name) ctx.syntaxNode();
-      if (isWithinImport(node)) {
+      Name name = (Name) ctx.syntaxNode();
+      if (isWithinImport(name)) {
         return;
       }
-      if (isQuestionablePropertyAccess(node)) {
-        ctx.addIssue(node, message());
+      if (isQuestionablePropertyAccess(name)) {
+        ctx.addIssue(name, message());
       }
     });
   }
@@ -78,13 +77,14 @@ protected boolean isException(CallExpression callExpression) {
   }
 
   private static boolean isQuestionablePropertyAccess(Name pyNameTree) {
-    Tree callExpression = TreeUtils.firstAncestorOfKind(pyNameTree, Tree.Kind.CALL_EXPR);
-    if (callExpression != null) {
-      // avoid raising twice the issue on call expressions like sys.stdin.read()
-      CallExpression call = (CallExpression) callExpression;
-      if (call.callee().descendants().anyMatch(tree -> tree == pyNameTree)) {
+    Tree parent = pyNameTree.parent();
+    while (parent != null && !parent.is(Tree.Kind.CALL_EXPR)) {
+      Tree grandParent = parent.parent();
+      if (grandParent != null && grandParent.is(Tree.Kind.CALL_EXPR) && ((CallExpression) grandParent).callee() == parent) {
+        // avoid raising twice the issue on call expressions like sys.stdin.read()
         return false;
       }
+      parent = grandParent;
     }
     Symbol symbol = pyNameTree.symbol();
     return symbol != null && questionablePropertyAccess.contains(symbol.fullyQualifiedName());

python-checks/src/test/java/org/sonar/python/checks/ExpressionsTest.java

@@ -21,16 +21,18 @@
 
 import com.sonar.sslr.impl.Parser;
 import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.stream.Collectors;
 import org.junit.Test;
 import org.sonar.python.PythonConfiguration;
 import org.sonar.python.api.tree.Expression;
 import org.sonar.python.api.tree.FileInput;
 import org.sonar.python.api.tree.Name;
+import org.sonar.python.api.tree.Tree;
 import org.sonar.python.api.tree.Tree.Kind;
 import org.sonar.python.parser.PythonParser;
 import org.sonar.python.semantic.SymbolTableBuilder;
+import org.sonar.python.tree.BaseTreeVisitor;
 import org.sonar.python.tree.PythonTreeMaker;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -75,12 +77,20 @@ public void falsy() {
   }
 
   private Expression exp(String code) {
-    FileInput tree = parse(code);
-    return tree.descendants()
-      .filter(Expression.class::isInstance)
-      .map(Expression.class::cast)
-      .findFirst()
-      .get();
+    return exp(parse(code));
+  }
+
+  private Expression exp(Tree tree) {
+    if (tree instanceof Expression) {
+      return (Expression) tree;
+    }
+    for (Tree child : tree.children()) {
+      Expression exp = exp(child);
+      if (exp != null) {
+        return exp;
+      }
+    }
+    return null;
   }
 
   private FileInput parse(String code) {
@@ -100,10 +110,19 @@ public void singleAssignedValue() {
   private Expression lastNameValue(String code) {
     FileInput root = parse(code);
     new SymbolTableBuilder().visitFileInput(root);
-    List<Name> names = root.descendants(Kind.NAME)
-      .map(Name.class::cast)
-      .collect(Collectors.toList());
+    NameVisitor nameVisitor = new NameVisitor();
+    root.accept(nameVisitor);
+    List<Name> names = nameVisitor.names;
     return Expressions.singleAssignedValue(names.get(names.size() - 1));
   }
 
+  private static class NameVisitor extends BaseTreeVisitor {
+    private List<Name> names = new ArrayList<>();
+
+    @Override
+    public void visitName(Name pyNameTree) {
+      names.add(pyNameTree);
+    }
+  }
+
 }

python-checks/src/test/resources/checks/emptyNestedBlock.py

@@ -35,6 +35,11 @@
 if 3 > 2:
     pass  # nothing to do
 
+if 3 > 2:
+   # no issue
+   pass
+elif k == 5: # unknown
+    pass
 
 def empty_function():
     pass