SONARJAVA-5149 S1075: find when uri is short and relative and stop raising issues #4961
Conversation
hashicorp-vault-sonar-prod
bot
changed the title
| @@ -45,8 +45,8 @@ void foo(String s, String var) throws URISyntaxException { | |||
| // ^^^^^^^^^^ | |||
|
|
|||
| String filename; | |||
| String path = "/home/path/to/my/file.txt"; // Noncompliant {{Refactor your code to get this URI from a customizable parameter.}} | |||
| // ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | |||
| String path_ = "/home/path/to/my/file.txt"; // Noncompliant {{Refactor your code to get this URI from a customizable parameter.}} | |||
There was a problem hiding this comment.
Why change this?
There was a problem hiding this comment.
it is not detected anymore otherwise, but I will do it differently don't hesitate to ask a question if you are curious
| static String staticRelativePath = "/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 path = "/a-great/path/for-this-example"; // FN, we don't test ot what refer an identifier when collecting them in annotations |
| static final String relativePath2 = "/group/members"; | ||
| static final String longRelativePath = "/group/members/list.json"; // Noncompliant | ||
| static final String urlPath = "https://www.mywebsite.com"; // Noncompliant | ||
| final String finalRelativePath = "/search"; // Noncompliant |
There was a problem hiding this comment.
Add a comment above that it's non-compliant, because static is missing
| // 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<>(); | ||
| private final ArrayList<VariableTree> hardCodedUri = new ArrayList<>(); |
There was a problem hiding this comment.
Let's always use interfaces on the left (List instead of ArrayList)
| } | ||
|
|
There was a problem hiding this comment.
delete empty line
| private final Deque<AnnotationTree> annotationsStack = new ArrayDeque<>(); | ||
| private final Set<String> variablesUsedInAnnotations = new HashSet<>(); | ||
|
|
||
| // use semantic ??? |
There was a problem hiding this comment.
This looks like a TODO and I don't really understand what it means. Could you try to change the comment please?
| return; | ||
| } | ||
|
|
||
| ModifiersTree modifiers = tree.modifiers(); |
There was a problem hiding this comment.
I think this could be simplified (or at least the following code passes tests):
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;
}
if (isHardcodedURI(initializer)) {
hardCodedUri.add(new HardCodedVariable(tree.simpleName().name(), initializer));
}
| .map(IdentifierData::identifier) | ||
| .collect(Collectors.toSet()); | ||
|
|
||
| hardCodedUri.stream() |
There was a problem hiding this comment.
Complex expressions are harder to read than simple if-then-else. I played in the IDE and came up with the following:
for(VariableData v: hardCodedUri) {
// equals to an identifier with unknown semantic, we cannot compare their symbols
if(idNamesWithoutSemantic.contains(v.identifier())) {
continue;
}
// idNamesWithSemantic is used to only compare the symbols when their string identifier are the same
// as comparing symbols is costly
if(idNamesWithSemantic.contains(v.identifier()) && idSymbols.contains(v.symbol())) {
continue;
}
reportHardcodedURI(v.initializer());
}
How about changing the code?
|
SONARJAVA-5149