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).

@bot-gradle

This comment has been minimized.

Sign in to view
@donat donat force-pushed the donat/problems/java-compiler-ids branch from 7125984 to 6bd7ff1 Compare February 13, 2025 10:24
@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 marked this pull request as ready for review February 14, 2025 14:20
@donat donat requested review from a team as code owners February 14, 2025 14:20
@donat donat requested review from octylFractal and wolfs February 14, 2025 14:20
@cobexer cobexer removed the request for review from a team February 17, 2025 09:25
@octylFractal octylFractal removed request for a team and octylFractal February 19, 2025 07:04
@donat donat requested a review from reinsch82 February 26, 2025 13:00
@donat donat added this to the 8.14 RC1 milestone Feb 26, 2025
@donat donat modified the milestones: 8.14 RC1, 9.0 Apr 1, 2025
@donat donat force-pushed the donat/problems/java-compiler-ids branch from 6bd7ff1 to d3136e0 Compare April 1, 2025 12:21
@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

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 from 19bf8f9 to 3829303 Compare April 3, 2025 09:07
@donat donat changed the base branch from release to bhegyi/problems/java-compiler-problem-contextualLabel April 3, 2025 09:07
@donat donat force-pushed the bhegyi/problems/java-compiler-problem-contextualLabel branch from 8b06157 to 60cce51 Compare April 3, 2025 09:12
@donat

This comment has been minimized.

Sign in to view
@bot-gradle

This comment has been minimized.

Sign in to view
@bot-gradle
Copy link
Collaborator

The following builds have passed:

@reinsch82 reinsch82 force-pushed the bhegyi/problems/java-compiler-problem-contextualLabel branch from 37f107b to 04ce32d Compare April 3, 2025 14:27
@donat donat force-pushed the donat/problems/java-compiler-ids branch from 3829303 to 4436936 Compare April 3, 2025 14:42
@donat
Copy link
Member Author

donat commented Apr 3, 2025

@bot-gradle test this

@bot-gradle

This comment has been minimized.

Sign in to view
@reinsch82 reinsch82 force-pushed the bhegyi/problems/java-compiler-problem-contextualLabel branch from 04ce32d to 0664c2f Compare April 3, 2025 15:01
@donat donat force-pushed the donat/problems/java-compiler-ids branch from 4436936 to adffc16 Compare April 3, 2025 15:13
@bot-gradle
Copy link
Collaborator

The following builds have failed:

@reinsch82 reinsch82 force-pushed the bhegyi/problems/java-compiler-problem-contextualLabel branch from 0664c2f to 832838f Compare April 3, 2025 17:35
Base automatically changed from bhegyi/problems/java-compiler-problem-contextualLabel to release April 4, 2025 07:22
@donat donat force-pushed the donat/problems/java-compiler-ids branch from adffc16 to ac4852b Compare April 4, 2025 08:31
@donat donat added this pull request to the merge queue Apr 4, 2025
@bot-gradle
Copy link
Collaborator

WARN: The milestone of this PR is 9.0, but the base branch version is 8.14

@donat donat modified the milestones: 9.0, 8.14 RC1 Apr 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 4, 2025
@cobexer cobexer added this pull request to the merge queue Apr 4, 2025
Merged via the queue into release with commit 306b6dd Apr 4, 2025
10 of 12 checks passed
@cobexer cobexer deleted the donat/problems/java-compiler-ids branch April 4, 2025 11:40
@github-actions github-actions bot mentioned this pull request Apr 4, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@reinsch82 reinsch82 reinsch82 approved these changes

@wolfs wolfs Awaiting requested review from wolfs

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

8.14 RC1

Development

Successfully merging this pull request may close these issues.

Java compilation errors and warnings have generic display names in Problems summary html

5 participants

@donat @bot-gradle @wolfs @reinsch82 @cobexer