Skip to content

Reverting some message logging changes in JavaToolMonitor #301

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

Merged
VISTALL merged 1 commit into from
Jun 24, 2026
Merged

Reverting some message logging changes in JavaToolMonitor #301

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 16 additions & 42 deletions ...-compiler-impl/src/main/java/consulo/java/compiler/impl/javaCompiler/JavaToolMonitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import consulo.application.util.concurrent.AppExecutorUtil;
import consulo.compiler.CacheCorruptedException;
import consulo.compiler.CompileContext;
import consulo.compiler.CompilerMessageCategory;

@github-actions github-actions Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice cleanup — collapsing logInfo/logError/logWarning into a single logMessage(category, …) and routing through newMessage(category, …).optionalUrl(fileUrl) preserves the original URI→VirtualFile resolution while removing the triplicated builder code. The MessageBuilderWrapper class is fully removed in this diff, so the dead-code concern is already handled. 👍

One small leftover: now that MessageBuilderWrapper.navigatable(Navigatable) is gone, the import consulo.navigation.Navigatable; (line 12) is unused and can be removed — it has no other reference in this file. (That line is outside the diff hunks, so a one-click suggestion block cannot be attached here.)

import consulo.compiler.localize.CompilerLocalize;
import consulo.java.rt.common.compiler.JavaCompilerInterface;
import consulo.localize.LocalizeValue;
Expand Down Expand Up @@ -114,26 +115,17 @@ public void dispose() {

@Override
public void logInfo(String message, String fileUri, long lineNumber, long columnNumber) throws TException {
new MessageBuilderWrapper(myCompileContext.newInfo(LocalizeValue.of(message)))
.url(fileUri)
.position((int) lineNumber, (int) columnNumber)
.add();
logMessage(CompilerMessageCategory.INFORMATION, message, fileUri, lineNumber, columnNumber);
}

@Override
public void logError(String message, String fileUri, long lineNumber, long columnNumber) throws TException {
new MessageBuilderWrapper(myCompileContext.newError(LocalizeValue.of(message)))
.url(fileUri)
.position((int) lineNumber, (int) columnNumber)
.add();
logMessage(CompilerMessageCategory.ERROR, message, fileUri, lineNumber, columnNumber);
}

@Override
public void logWarning(String message, String fileUri, long lineNumber, long columnNumber) throws TException {
new MessageBuilderWrapper(myCompileContext.newWarning(LocalizeValue.of(message)))
.url(fileUri)
.position((int) lineNumber, (int) columnNumber)
.add();
logMessage(CompilerMessageCategory.WARNING, message, fileUri, lineNumber, columnNumber);
}

@Override
Expand Down Expand Up @@ -176,39 +168,21 @@ public void fileWrote(String filePath) throws TException {
// }
// }

private class MessageBuilderWrapper implements CompileContext.MessageBuilder {
private final CompileContext.MessageBuilder myDelegate;

private MessageBuilderWrapper(CompileContext.MessageBuilder delegate) {
myDelegate = delegate;
}

@Override
public CompileContext.MessageBuilder url(String url) {
try {
VirtualFile fileByURL = VirtualFileUtil.findFileByURL(new URI(url).toURL());
if (fileByURL != null) {
myDelegate.url(fileByURL.getUrl());
}
}
catch (Exception ignored) {
private void logMessage(CompilerMessageCategory category, String message, String fileUri, long lineNumber, long columnNumber) {

@github-actions github-actions Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Extracting a shared logMessage(...) helper is a nice cleanup, and the URI→VirtualFile resolution is correctly preserved here (now feeding optionalUrl, which tolerates a null).

One follow-up: this new method inlines exactly what MessageBuilderWrapper.url(...) used to do. With logInfo/logError/logWarning no longer constructing it, the inner MessageBuilderWrapper class (lines 171–205) now has no remaining new MessageBuilderWrapper(...) call sites anywhere in the codebase — it is dead code and can be deleted in this PR.

(Those lines are outside this diff hunk, so a one-click suggestion cannot be attached; please remove the class manually.)

String fileUrl = null;
try {
URI uri = new URI(fileUri);
VirtualFile fileByURL = VirtualFileUtil.findFileByURL(uri.toURL());
if (fileByURL != null) {
fileUrl = fileByURL.getUrl();
}
return this;
}

@Override
public CompileContext.MessageBuilder position(int line, int column) {
return myDelegate.position(line, column);
catch (Exception ignored) {
}

@Override
public CompileContext.MessageBuilder navigatable(Navigatable navigatable) {
return myDelegate.navigatable(navigatable);
}

@Override
public void add() {
myDelegate.add();
}
myCompileContext.newMessage(category, LocalizeValue.of(message))
.optionalUrl(fileUrl)
.position((int) lineNumber, (int) columnNumber)
.add();
}
}
Loading