SONARJAVA-5158 update issue message #4964
Conversation
erwan-serandour
requested review from
dorian-burihabwa-sonarsource
and removed request for
dorian-burihabwa-sonarsource
| String issueMessage = MessageFormat.format(MESSAGE, javaFile.getName(), packageName, dir); | ||
| int srcIndex = dir.indexOf("src"); | ||
| String truncatedPath = dir.substring(srcIndex == -1 ? 0 : srcIndex); | ||
| String issueMessage = MessageFormat.format(MESSAGE, truncatedPath, packageName.replace(File.separator, ".")); |
There was a problem hiding this comment.
Can we replace MessageFormat and use String.format? Or is there a specific reason to use the former?
| int srcIndex = dir.indexOf("src"); | ||
| String truncatedPath = dir.substring(srcIndex == -1 ? 0 : srcIndex); |
There was a problem hiding this comment.
I understand that by convention, java files should be placed under src/{main|test}/java, but users can customize the location where their sources are located. Can we use the module specific source directory to match?
There was a problem hiding this comment.
After discussion, a simpler approach is to use sonar.projectBaseDir as the base root
|
|
| @Test | ||
| void mismatch_with_root_directory() { | ||
| CheckVerifier.newVerifier() | ||
| .onFile("src/test/files/checks/mismatchPackage/Mismatch.java") |
There was a problem hiding this comment.
I think this might be a good time to move the test files to the java-checks-test-sources module
| }); | ||
|
|
||
| assertThat(e) | ||
| .isInstanceOf(UnsupportedOperationException.class); |
There was a problem hiding this comment.
it might be worth checking the exception message here
...-checks-testkit/src/main/java/org/sonar/java/checks/verifier/internal/JavaCheckVerifier.java
Show resolved
Hide resolved
| @Rule(key = "VerifyProjectLevelWorkDir") | ||
| protected static final class VerifyProjectLevelWorkDir implements JavaFileScanner { | ||
| private final String projectLevelWorkDir; | ||
|
|
||
| public VerifyProjectLevelWorkDir(String projectLevelWorkDir){ | ||
| this.projectLevelWorkDir = projectLevelWorkDir; | ||
| } | ||
|
|
||
| @Override | ||
| public void scanFile(JavaFileScannerContext context) { | ||
| if(!projectLevelWorkDir.equals(context.getRootProjectWorkingDirectory().getPath())){ | ||
| throw new RuntimeException("This checks fails if project level does not match context.getRootProjectWorkingDirectory"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
What is this class and where is it used in the code?
There was a problem hiding this comment.
This is a mock java check with the purpose to test that property root project directory corresponds to what is expected. But indeed the message and the variable name is confusing.
| String rootName = rootDirectory.getName(); | ||
| List<String> path = new ArrayList<>(); | ||
|
|
||
| while (fileDirectory != null && !fileDirectory.getName().equals(rootName)) { | ||
| path.add(0, fileDirectory.getName()); | ||
| fileDirectory = fileDirectory.getParentFile(); | ||
| } | ||
|
|
||
| truncatedPath = String.join(File.separator, path); |
There was a problem hiding this comment.
We should extract part of the code could leave in its own method. Right now, it is using and re-assigning variables defined outside of the try block, which are also used outside of the try block
|
This PR is stale because it has been open 7 days with no activity. If there is no activity in the next 7 days it will be closed automatically |
|
SONARJAVA-5158
Part of