Skip to content

Use diagnostic message as the display name of the Java compilation problem ids #32411

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 4, 2025
Merged

Use diagnostic message as the display name of the Java compilation problem ids #32411

merged 1 commit into from
Apr 4, 2025

Conversation

donat
Copy link
Member

@donat donat commented Feb 12, 2025 •
edited
Loading

Fixes #32915

Context

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@donat

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@donat donat force-pushed the donat/problems/java-compiler-ids branch 2 times, most recently from 83ff440 to 7125984 Compare February 12, 2025 13:37
@donat

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
wolfs
wolfs reviewed Feb 12, 2025
View reviewed changes
...ge-java/src/main/java/org/gradle/api/internal/tasks/compile/DiagnosticToProblemListener.java Outdated
private static void addDetails(ProblemSpec spec, Diagnostic<? extends JavaFileObject> diagnostic) {
String message = diagnostic.getMessage(Locale.getDefault());
private static void addContextualLabel(ProblemSpec spec, Diagnostic<? extends JavaFileObject> diagnostic) {
String message = diagnosticMessage(diagnostic);
Copy link
Member

Choose a reason for hiding this comment

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

🤔 If the message is null, wouldn't it make more sense to not set the contextual label instead of using Unknown?

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed.

...ge-java/src/main/java/org/gradle/api/internal/tasks/compile/DiagnosticToProblemListener.java Outdated

private void addFormattedMessage(ProblemSpec spec, Diagnostic<? extends JavaFileObject> diagnostic) {
private void addDetails(ProblemSpec spec, Diagnostic<? extends JavaFileObject> diagnostic) {
String formatted = toFormattedMessage(diagnostic);
Copy link
Member

Choose a reason for hiding this comment

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

🤔 What is the difference between the formatted message and the message? Would it make sense to drop the first line of the formatted message to use it as details given that the first line is already part of the contextual label? While still printing out the full formatted message to the console?

Copy link
Member Author

Choose a reason for hiding this comment

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

That kind of makes sense, let me play around with that.

Copy link
Member Author

Choose a reason for hiding this comment

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

the message: is the short summary of the problem, eg ';' expected
the formatted message is the actual compiler-specific output you see on the console, eg

  /Users/donat/Development/git/gradle/gradle-feature/platforms/jvm/language-java/build/tmp/teŝt files/JavaMultiCo.Test/ziu9r/project2/src/main/java/Project2.java:5: error: ';' expected
      String s = "Hello, World!"
                                ^

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so the first line of the formatted message seems to duplicate the severity, the location, and the message, right? Is that always the case or are there some corner cases when this does not happen? If it is always the case I'd drop the first line for the problems API details from the formatted message.

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that always the case or are there some corner cases when this does not happen?

We don't know exactly. The actual content of the formatted message is unknown, because it is an implementation detail of the Java compiler.
Also, there are some known cases, where we cannot even get the formatted message and we know we use the fallback. For example when Java 8 is used with annotation processing (see #31160).