Skip to content

Commit

Permalink
SONARPY-862 S117 (LocalVariableAndParameterNameConventionCheck) shoul…
Browse files Browse the repository at this point in the history
…dn't raise on type aliases (#2064)
  • Loading branch information
thomas-serre-sonarsource authored Oct 11, 2024
1 parent c0e14aa commit 83dbc83
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@

import java.util.Comparator;
import java.util.EnumSet;
import java.util.List;
import java.util.regex.Pattern;
import org.sonar.check.Rule;
import org.sonar.check.RuleProperty;
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
import org.sonar.plugins.python.api.SubscriptionContext;
import org.sonar.plugins.python.api.symbols.Symbol;
import org.sonar.plugins.python.api.symbols.Usage;
import org.sonar.plugins.python.api.tree.AssignmentStatement;
import org.sonar.plugins.python.api.tree.Expression;
import org.sonar.plugins.python.api.tree.FunctionDef;
import org.sonar.plugins.python.api.tree.Name;
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
import org.sonar.plugins.python.api.tree.Tree;
import org.sonar.plugins.python.api.symbols.Symbol;
import org.sonar.plugins.python.api.symbols.Usage;

@Rule(key = "S117")
public class LocalVariableAndParameterNameConventionCheck extends PythonSubscriptionCheck {
Expand Down Expand Up @@ -78,7 +82,42 @@ private void checkName(Symbol symbol, SubscriptionContext ctx) {
}

private static boolean isType(Symbol symbol) {
return symbol.usages().stream().map(Usage::tree).filter(Expression.class::isInstance).map(Expression.class::cast).anyMatch(e -> e.type().mustBeOrExtend("type"));
return isExtendingType(symbol) || isAssignedFromTyping(symbol);
}

private static boolean isExtendingType(Symbol symbol) {
return symbol.usages().stream().map(Usage::tree).filter(Expression.class::isInstance).map(Expression.class::cast).anyMatch(e -> e.type().mustBeOrExtend("type")) ||
(symbol.annotatedTypeName() != null && symbol.annotatedTypeName().startsWith("typing."));
}

private static boolean isAssignedFromTyping(Symbol symbol) {
List<Tree> assignmentNames = symbol.usages().stream().filter(u -> u.kind() == Usage.Kind.ASSIGNMENT_LHS).map(Usage::tree).toList();
for (Tree assignmentName : assignmentNames) {
Expression assignedValue = getAssignedValue(assignmentName);
if (assignedValue == null) {
continue;
}
if (assignedValue.is(Tree.Kind.SUBSCRIPTION)) {
SubscriptionExpression subscriptionExpression = (SubscriptionExpression) assignedValue;
if (subscriptionExpression.object().is(Tree.Kind.NAME)) {
Symbol assignedSymbol = ((Name) subscriptionExpression.object()).symbol();
if (assignedSymbol != null && isExtendingType(assignedSymbol)) {
return true;
}
}
}
}
return false;
}

private static Expression getAssignedValue(Tree assignmentName) {
while (assignmentName != null && !assignmentName.is(Tree.Kind.ASSIGNMENT_STMT)) {
assignmentName = assignmentName.parent();
}
if (assignmentName == null) {
return null;
}
return ((AssignmentStatement) assignmentName).assignedValue();
}

private void raiseIssueForNameAndUsage(SubscriptionContext ctx, String name, Usage usage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ def type_variables_ok(MyTypeParameter: type):
from typing import NamedTuple
Employee = NamedTuple('Employee', [('name', str), ('id', int)])

def type_aliases_from_typing_special_form():
from typing import Type, Union
class A: ...
MyType = Type[A] # OK
AliasType = Union[str, int] # OK

# Assigned from a subscription check, but not from a typing_SpecialForm
array = [1,2,3]
MyInt = array[1] # Noncompliant
MyOtherInt = [1,2,3][1] # Noncompliant

def type_variables_fp():
class MyClass:
Expand Down

0 comments on commit 83dbc83

Please sign in to comment.