Skip to content

Commit

Permalink
SONARPHP-1577 S4144 should not report an issue on method/function usi…
Browse files Browse the repository at this point in the history 
…ng the __FUNCTION__ constant
  • Loading branch information
@rudy-regazzoni-sonarsource
rudy-regazzoni-sonarsource committed Jan 6, 2025
1 parent 3852234 commit 96b3101
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 9 deletions.
26 changes: 20 additions & 6 deletions php-checks/src/main/java/org/sonar/php/checks/DuplicatedMethodCheck.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.sonar.plugins.php.api.tree.declaration.ReturnTypeClauseTree;
import org.sonar.plugins.php.api.tree.expression.AnonymousClassTree;
import org.sonar.plugins.php.api.tree.expression.ExpressionTree;
import org.sonar.plugins.php.api.tree.expression.LiteralTree;
import org.sonar.plugins.php.api.tree.expression.NameIdentifierTree;
import org.sonar.plugins.php.api.tree.lexical.SyntaxToken;
import org.sonar.plugins.php.api.tree.statement.BlockTree;
Expand All @@ -52,9 +53,11 @@ public class DuplicatedMethodCheck extends PHPVisitorCheck {
private static final Function<FunctionTree, NameIdentifierTree> METHOD_TO_NAME = f -> ((MethodDeclarationTree) f).name();
private static final Function<FunctionTree, NameIdentifierTree> FUNCTION_TO_NAME = f -> ((FunctionDeclarationTree) f).name();
private static final int MINIMUM_NUMBER_OF_STATEMENTS = 2;
private static final Set<String> EXCLUDED_MAGIC_CONSTANTS = Set.of("__FUNCTION__", "__LINE__", "__METHOD__");

private final Deque<List<MethodDeclarationTree>> methods = new LinkedList<>();
private final List<FunctionDeclarationTree> functions = new ArrayList<>();
private final Deque<Boolean> scopeContainMagicConstantExclusion = new LinkedList<>();

@Override
public void visitCompilationUnit(CompilationUnitTree tree) {
Expand All @@ -66,20 +69,22 @@ public void visitCompilationUnit(CompilationUnitTree tree) {

@Override
public void visitFunctionDeclaration(FunctionDeclarationTree tree) {
// Ignore functions with fewer than 2 statements
if (tree.body().statements().size() >= MINIMUM_NUMBER_OF_STATEMENTS) {
scopeContainMagicConstantExclusion.addFirst(false);
super.visitFunctionDeclaration(tree);
// Ignore functions with exceptions + ensure that the function has the minimum number of statements
if (Boolean.FALSE.equals(scopeContainMagicConstantExclusion.pop()) && tree.body().statements().size() >= MINIMUM_NUMBER_OF_STATEMENTS) {
functions.add(tree);
}
super.visitFunctionDeclaration(tree);
}

@Override
public void visitMethodDeclaration(MethodDeclarationTree tree) {
// Ignore abstract and empty methods
if (isDuplicateCandidate(tree)) {
scopeContainMagicConstantExclusion.addFirst(false);
super.visitMethodDeclaration(tree);
// Ignore functions with exceptions + abstract and empty methods
if (Boolean.FALSE.equals(scopeContainMagicConstantExclusion.pop()) && isDuplicateCandidate(tree)) {
methods.peek().add(tree);
}
super.visitMethodDeclaration(tree);
}

private static boolean isDuplicateCandidate(MethodDeclarationTree tree) {
Expand Down Expand Up @@ -118,6 +123,15 @@ public void visitAnonymousClass(AnonymousClassTree tree) {
checkDuplications(methods.pop(), METHOD_TO_NAME);
}

@Override
public void visitLiteral(LiteralTree tree) {
if (!scopeContainMagicConstantExclusion.isEmpty() && tree.is(Tree.Kind.MAGIC_CONSTANT) && EXCLUDED_MAGIC_CONSTANTS.contains(tree.value())) {
scopeContainMagicConstantExclusion.pop();
scopeContainMagicConstantExclusion.addFirst(true);
}
super.visitLiteral(tree);
}

private void checkDuplications(List<? extends FunctionTree> functionDeclarations, Function<FunctionTree, NameIdentifierTree> toName) {
Set<? super FunctionTree> reported = new HashSet<>();
for (int i = 0; i < functionDeclarations.size(); i++) {
Expand Down
36 changes: 33 additions & 3 deletions php-checks/src/test/resources/checks/DuplicatedMethodCheck.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@ function fun5($a, $c) {
function fun6($a, $c) { // OK, not raising if there is less than 2 statements
echo $a;
}
function fun7($a, $b) {
echo $a;
echo $b;
echo __FUNCTION__;
}
function fun8($a, $b) { // OK, not raising when constant __FUNCTION__ is used
echo $a;
echo $b;
echo __FUNCTION__;
}
function fun9($a, $b) {
echo $a;
echo $b;
echo __DIR__;
}
function fun10($a, $b) { // Noncompliant {{Update this method so that its implementation is not identical to "fun9" on line 33.}}
echo $a;
echo $b;
echo __DIR__;
}

class A {
public $var = 'a default value';
Expand All @@ -38,7 +58,7 @@ public function displayVar() {
echo $this->var;
}

public function displayVar2() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 35.}}
public function displayVar2() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 55.}}
// ^^^^^^^^^^^
echo $this->var;
echo $this->var;
Expand All @@ -59,11 +79,11 @@ public function displayVar() {
echo $this->var;
echo $this->var;
}
public function displayVar2() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 58.}}
public function displayVar2() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 78.}}
echo $this->var;
echo $this->var;
}
public function displayVar3() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 58.}}
public function displayVar3() { // Noncompliant {{Update this method so that its implementation is not identical to "displayVar" on line 78.}}
echo $this->var;
echo $this->var;
}
Expand All @@ -73,6 +93,16 @@ public function displayVarSmall() {
public function displayVarSmall2() { // OK, not raising if there is less than 2 statements
echo $this->var;
}
public function displayVarMagicConstant1() {
echo $this->var;
echo $this->var;
echo __FUNCTION__;
}
public function displayVarMagicConstant2() { // OK, not raising when constant __FUNCTION__ is used
echo $this->var;
echo $this->var;
echo __FUNCTION__;
}
};
}

Expand Down

0 comments on commit 96b3101

Please sign in to comment.