Skip to content

Commit cbd1cb7

Browse files
marc-jasper-sonarsourcesonartech
authored andcommitted
SONARPY-1845 S117: Fix FP when parameter name is coming from superclass method declaration (#735)
GitOrigin-RevId: 0666f59db571b3179c21a5cae3eb18207df4dede
1 parent ac149f8 commit cbd1cb7

File tree

2 files changed

+80
-0
lines changed

2 files changed

+80
-0
lines changed

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

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@
2727
import org.sonar.check.RuleProperty;
2828
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
2929
import org.sonar.plugins.python.api.SubscriptionContext;
30+
import org.sonar.plugins.python.api.symbols.FunctionSymbol;
3031
import org.sonar.plugins.python.api.symbols.Symbol;
3132
import org.sonar.plugins.python.api.symbols.Usage;
3233
import org.sonar.plugins.python.api.tree.AssignmentStatement;
3334
import org.sonar.plugins.python.api.tree.Expression;
3435
import org.sonar.plugins.python.api.tree.FunctionDef;
3536
import org.sonar.plugins.python.api.tree.Name;
37+
import org.sonar.plugins.python.api.tree.Parameter;
3638
import org.sonar.plugins.python.api.tree.SubscriptionExpression;
3739
import org.sonar.plugins.python.api.tree.Tree;
3840
import org.sonar.plugins.python.api.types.v2.PythonType;
@@ -168,7 +170,52 @@ private void raiseIssueForNameAndUsage(SubscriptionContext ctx, String name, Usa
168170
if (name.length() <= 1) {
169171
return;
170172
}
173+
} else if (kind == UsageV2.Kind.PARAMETER && isParameterNameFromOverriddenMethod(usage, name)) {
174+
return;
171175
}
172176
ctx.addIssue(usage.tree(), String.format(MESSAGE, type, name, format));
173177
}
178+
179+
private static boolean isParameterNameFromOverriddenMethod(UsageV2 usage, String parameterName) {
180+
Tree functionLikeAncestor = TreeUtils.firstAncestorOfKind(usage.tree(), Tree.Kind.FUNCDEF, Tree.Kind.LAMBDA);
181+
if (functionLikeAncestor == null || functionLikeAncestor.is(Tree.Kind.LAMBDA)) {
182+
return false;
183+
}
184+
FunctionDef functionDef = (FunctionDef) functionLikeAncestor;
185+
186+
int parameterIndex = getParameterIndex(functionDef, parameterName);
187+
if (parameterIndex < 0) {
188+
return false;
189+
}
190+
191+
FunctionSymbol functionSymbol = TreeUtils.getFunctionSymbolFromDef(functionDef);
192+
if (functionSymbol == null) {
193+
return false;
194+
}
195+
196+
List<FunctionSymbol> overriddenMethods = SymbolUtils.getOverriddenMethods(functionSymbol);
197+
for (FunctionSymbol overriddenMethod : overriddenMethods) {
198+
List<FunctionSymbol.Parameter> overriddenParams = overriddenMethod.parameters();
199+
// parameterIndex is based on positional parameters only, while overriddenParams contains all parameters.
200+
// This comparison works because positional parameters are always a prefix of the full parameter list.
201+
if (parameterIndex < overriddenParams.size()) {
202+
String overriddenParamName = overriddenParams.get(parameterIndex).name();
203+
if (parameterName.equals(overriddenParamName)) {
204+
return true;
205+
}
206+
}
207+
}
208+
return false;
209+
}
210+
211+
private static int getParameterIndex(FunctionDef functionDef, String parameterName) {
212+
List<Parameter> parameters = TreeUtils.positionalParameters(functionDef);
213+
for (int i = 0; i < parameters.size(); i++) {
214+
Name paramName = parameters.get(i).name();
215+
if (paramName != null && parameterName.equals(paramName.name())) {
216+
return i;
217+
}
218+
}
219+
return -1;
220+
}
174221
}

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,36 @@ def ready(self):
112112

113113
from django.apps import apps
114114
Product = apps.get_model('shop', 'Product')
115+
116+
117+
import json
118+
from typing import Any
119+
120+
class CustomJSONEncoder(json.JSONEncoder):
121+
122+
def default(self, o: Any) -> dict[str, Any]: # Compliant: `o` parameter name comes from json.JSONEncoder.default
123+
return {"value": str(o)}
124+
125+
126+
class EdgeCase:
127+
method = None
128+
129+
def method(self, ID): # Noncompliant
130+
pass
131+
132+
133+
class BaseClass:
134+
def process(self, data_input):
135+
pass
136+
137+
class DerivedClass(BaseClass):
138+
def process(self, ID): # Noncompliant
139+
pass
140+
141+
class BaseShortSignature:
142+
def execute(self):
143+
pass
144+
145+
class DerivedLongerSignature(BaseShortSignature):
146+
def execute(self, ExtraParam): # Noncompliant
147+
pass

0 commit comments

Comments
 (0)