Skip to content

Commit

Permalink
fix FN
Browse files Browse the repository at this point in the history
  • Loading branch information
erwan-serandour committed Dec 13, 2024
1 parent ca50526 commit 2d1ee6b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 28 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package checks;

public class HardcodedURICheckSample {

String path = "/home/path/to/my/file.txt"; // Noncompliant

String aVarPath = "/home/path/to/my/file.txt"; // FN, missing semantics
@MyAnnotation(aVarPath = "")
int x = 0;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class HardcodedURICheckSample {
String stuff() default "none";
// we cannot name method path otherwise path is detected as an identifier used in annotation
//, and it creates clashes (FN) with path variables or fields
String path__() default "/";
String path() default "/";
}

static final String PATH_WITH_EXPANSION_PATTERN = "/.*+\\.[a-z0-9]{2,4}$"; // Compliant
Expand All @@ -20,13 +20,13 @@ class HardcodedURICheckSample {
String fileName = "//my-network-drive/folder/file.txt"; // Noncompliant
String[] stuffs = new String[1];

@MyAnnotation(stuff = "yolo", path__ = "/{var}/bulu/stuff") // Compliant - annotations are ignored
@MyAnnotation(stuff = "yolo", path = "/{var}/bulu/stuff") // Compliant - annotations are ignored
void bar(String var) { }

@MyAnnotation(stuff = "/{var}/bulu/stuff") // Compliant - not a path assignmnet
void qix(String var) { }

@MyAnnotation(path__ = "/{var}/bulu/stuff") // Compliant - annotations are ignored
@MyAnnotation(path = "/{var}/bulu/stuff") // Compliant - annotations are ignored
void foo(String s, String var) throws URISyntaxException {
new Object();

Expand Down Expand Up @@ -77,7 +77,7 @@ void foo(String s, String var) throws URISyntaxException {
static String finalIsMissingPath = "/search"; // Noncompliant

static final String default_uri_path = "/a-great/path/for-this-example"; // Compliant, default_uri is constant and is used in an annotation
String aVar = "/a-great/path/for-this-example"; // FN, we don't test to what refer an identifier when collecting them in annotations
String aVarPath = "/a-great/path/for-this-example"; // Noncompliant

@MyAnnotation2(aVar = default_uri_path)
void annotated(){}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Deque;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.sonar.check.Rule;
Expand All @@ -34,6 +33,7 @@
import org.sonar.plugins.java.api.IssuableSubscriptionVisitor;
import org.sonar.plugins.java.api.JavaFileScannerContext;
import org.sonar.plugins.java.api.semantic.MethodMatchers;
import org.sonar.plugins.java.api.semantic.Symbol;
import org.sonar.plugins.java.api.tree.AnnotationTree;
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
import org.sonar.plugins.java.api.tree.BaseTreeVisitor;
Expand All @@ -43,7 +43,6 @@
import org.sonar.plugins.java.api.tree.LiteralTree;
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
import org.sonar.plugins.java.api.tree.Modifier;
import org.sonar.plugins.java.api.tree.ModifiersTree;
import org.sonar.plugins.java.api.tree.NewClassTree;
import org.sonar.plugins.java.api.tree.Tree;
import org.sonar.plugins.java.api.tree.VariableTree;
Expand Down Expand Up @@ -83,27 +82,49 @@ public class HardcodedURICheck extends IssuableSubscriptionVisitor {

// we use these variables to track when we are visiting an annotation
private final Deque<AnnotationTree> annotationsStack = new ArrayDeque<>();
private final Set<String> variablesUsedInAnnotations = new HashSet<>();

// use semantic ???
private record HardCodedVariable(String identifier, ExpressionTree initializer) {
private record IdentifierData(Symbol symbol, String identifier) {
}

private final List<HardCodedVariable> hardCodedUri = new ArrayList<>();
private final List<IdentifierData> identifiersUsedInAnnotations = new ArrayList<>();

private record VariableData(Symbol symbol, String identifier, ExpressionTree initializer) {
}

private final List<VariableData> hardCodedUri = new ArrayList<>();

@Override
public void setContext(JavaFileScannerContext context) {
super.setContext(context);
annotationsStack.clear();
variablesUsedInAnnotations.clear();
identifiersUsedInAnnotations.clear();
hardCodedUri.clear();
}

@Override
public void leaveFile(JavaFileScannerContext context) {
// now, we know all variable that are used in annotation so we can report issues
Set<String> idNames = identifiersUsedInAnnotations.stream()
.map(IdentifierData::identifier)
.collect(Collectors.toSet());
Set<Symbol> idSymbols = identifiersUsedInAnnotations.stream()
.map(IdentifierData::symbol)
.filter(s -> !s.isUnknown())
.collect(Collectors.toSet());
Set<String> idNamesWithoutSemantic = identifiersUsedInAnnotations.stream()
.filter(i -> i.symbol().isUnknown())
.map(IdentifierData::identifier)
.collect(Collectors.toSet());

hardCodedUri.stream()
.filter(v -> !variablesUsedInAnnotations.contains(v.identifier()))
.filter(v -> {
boolean insideAnnotation = idNames.contains(v.identifier())
&& (
// equals to an identifier with unknown semantic, we cannot compare their symbols
idNamesWithoutSemantic.contains(v.identifier())
|| idSymbols.contains(v.symbol()));
return !insideAnnotation;
})
.forEach(v -> reportHardcodedURI(v.initializer()));
}

Expand All @@ -122,8 +143,8 @@ public void visitNode(Tree tree) {
} else if (tree instanceof AnnotationTree annotationTree) {
annotationsStack.add(annotationTree);
} else if (tree instanceof IdentifierTree identifier && !annotationsStack.isEmpty()) {
variablesUsedInAnnotations.add(identifier.name());
} else if(tree instanceof AssignmentExpressionTree assignment){
identifiersUsedInAnnotations.add(new IdentifierData(identifier.symbol(), identifier.name()));
} else if (tree instanceof AssignmentExpressionTree assignment) {
checkAssignment(assignment);
}
}
Expand Down Expand Up @@ -152,19 +173,21 @@ private void checkVariable(VariableTree tree) {
return;
}

ModifiersTree modifiers = tree.modifiers();
Predicate<String> smallRelativeUri = stringValue ->
ModifiersUtils.hasModifier(modifiers, Modifier.STATIC)
&& ModifiersUtils.hasModifier(modifiers, Modifier.FINAL)
&& RELATIVE_URI_PATTERN.matcher(stringValue).matches();
String stringLiteral = stringLiteral(initializer);
if (stringLiteral == null) {
return;
}

// small relative Uri that are static and final are allowed
if (ModifiersUtils.hasAll(tree.modifiers(), Modifier.STATIC, Modifier.FINAL)
&& RELATIVE_URI_PATTERN.matcher(stringLiteral).matches()) {
return;
}

String stringLiteral = stringLiteral(initializer);
if (stringLiteral != null
&& !smallRelativeUri.test(stringLiteral)
&& isHardcodedURI(initializer)
) {
hardCodedUri.add(new HardCodedVariable(tree.simpleName().name(), initializer));
if (isHardcodedURI(initializer)) {
hardCodedUri.add(new VariableData(tree.symbol(),
tree.simpleName().name(),
initializer));
}
}

Expand Down Expand Up @@ -195,7 +218,6 @@ private void checkExpression(ExpressionTree expr) {
} else {
reportStringConcatenationWithPathDelimiter(expr);
}

}

private static boolean isHardcodedURI(ExpressionTree expr) {
Expand All @@ -210,7 +232,7 @@ private static boolean isHardcodedURI(ExpressionTree expr) {
private static String stringLiteral(ExpressionTree expr) {
ExpressionTree unquoted = ExpressionUtils.skipParentheses(expr);

if (unquoted instanceof LiteralTree literalTree && literalTree.is(Tree.Kind.STRING_LITERAL)) {
if (unquoted instanceof LiteralTree literalTree && literalTree.is(Tree.Kind.STRING_LITERAL)) {
return LiteralUtils.trimQuotes(literalTree.value());
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.sonar.java.checks.verifier.CheckVerifier;

import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath;
import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath;

class HardcodedURICheckTest {
@Test
Expand All @@ -30,4 +31,12 @@ void test() {
.verifyIssues();
}

@Test
void test_without_semantic() {
CheckVerifier.newVerifier()
.onFile(nonCompilingTestSourcesPath("checks/HardcodedURICheckSample.java"))
.withCheck(new HardcodedURICheck())
.verifyIssues();
}

}

0 comments on commit 2d1ee6b

Please sign in to comment.