Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Translate Diagnostic.Kind.NOTE to Security.HINT not WARNING #8163

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Jan 16, 2025

Ever since jenkinsci/stapler#575 I have been bothered by the NetBeans editor showing gratuitous yellow warning lines for a variety of annotation processor notes that are not in fact warnings. For example https://github.com/jenkinsci/workflow-job-plugin/blob/5b18defc07f27c7bde3521e04b7bab66398a89d3/src/main/java/org/jenkinsci/plugins/workflow/job/properties/DisableConcurrentBuildsJobProperty.java#L49-L50 displays warning just because the processor noted that it was generating a file in response to an annotation. Downgrading the “severity” to a “hint” still shows an icon in the gutter, which is OK, and still shows a “warning” in the Action Items window, which is less OK, but at least the editor does not make it look like I did something wrong.

Before:
Screenshot from 2025-01-16 14-45-31

After:
Screenshot from 2025-01-16 14-44-41

By the way VS Code using the oracle.oracle-java extension (23.0.1) also suffers from this:
Screenshot from 2025-01-16 14-58-54

@jglick
Copy link
Contributor Author

jglick commented Jan 16, 2025

@lahodaj lahodaj added the Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) label Jan 17, 2025
@apache apache locked and limited conversation to collaborators Jan 17, 2025
@apache apache unlocked this conversation Jan 17, 2025
Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me. We could also consider dropping NOTEs completely, as they typically do not carry much useful information.

Yes, I believe Diagnostics that are not associated with a file are ignored here.

@jglick
Copy link
Contributor Author

jglick commented Jan 17, 2025

CI is reporting a failure in

public void testMultipleCtors() throws Exception {
performFixTest("test/Test.java",
"package test;\n" +
"public class Test {\n" +
" private int a;\n" +
" private int c;\n" +
" public Test() {}\n" +
" public Test(int a, int |b, int c) {\n" +
" this.a = a;\n" +
" this.c = c;\n" +
" }\n" +
"}\n",
"5:28-5:28:hint:Unused Parameter",
"FixImpl:false",
"package test; public class Test { private int a; private int b; private int c; public Test() {} public Test(int a, int b, int c) { this.a = a; this.b = b; this.c = c; } } ");
}
which I do not get locally.

@matthiasblaesing
Copy link
Contributor

That test is known to be flaky. I restarted the test run for that test group.

@mbien mbien added this to the NB26 milestone Jan 24, 2025
@mbien
Copy link
Member

mbien commented Jan 25, 2025

CI is reporting a failure in

I extracted it into a separate issue #8195 since this has potential to be a real bug, test stability meta issue is #8183

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants