Refactoring compilation engine

[unv-unv]

No description provided.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 3m 0s —— View job

---

PR Review: Refactoring compilation engine

• Gather context (diff, changed files)
• Read repo guidelines (no AGENTS.md/CLAUDE.md present)
• Review changed files
• Post inline review comments

I reviewed all 11 changed files. The bulk of this PR is a clean mechanical reformat (tabs → 4-space indent, final cleanup, diamond operators) plus a genuine API migration from the old callback.message(...) / context.addMessage(...) calls to the new fluent newError/newWarning/newInfo/newMessage(...).…add() MessageBuilder API. That migration is consistent and reads well across OutputParser, CompilerParsingThread, BackendCompilerWrapper, FilePathActionJavac, and both JavacOutputParser variants.

I did find 3 bugs introduced by the migration, posted as inline suggestions you can apply with one click:

#	File	Line	Issue
1	JavaToolMonitor.java	191	Infinite recursion → StackOverflowError. MessageBuilderWrapper.url() calls its own url(...) instead of myDelegate.url(...); the resolved VirtualFile.getUrl() re-resolves to the same file forever. Every logInfo/logError/logWarning carrying a file URI hits this.
2	JavaCompiler.java	111	Compiler error swallowed — missing .add(). A CompilerException from wrapper.compile(...) is built but never reported, so a real failure only reaches idea.log and the user sees a silent "success".
3	JavacOutputParser.java	163	OOM message dropped — missing .add(). The java.lang.OutOfMemoryError branch builds an error that is never added.

The common thread for #2 and #3: the fluent API is lazy — building a MessageBuilder without .add() is a no-op, unlike the old addMessage(...) which reported immediately. It's worth grepping for any other new{Error,Warning,Info,Message}( not terminated by .add() (I checked the changed files; these two were the only stragglers, but the pattern is easy to repeat).

Minor notes (no inline comment needed)

• OutputParser.java and JavacOutputParser.java end without a trailing newline (\ No newline at end of file). Low priority, but worth fixing while touching these files.
• JavaToolMonitor previously fell back to adding a message with a null URL when the file couldn't be resolved; the new wrapper silently skips setting a URL (message still added) — behavior is equivalent, just noting it's intentional.

Nice cleanup overall — once the three .add()/delegation fixes land this is good to go.
· branch refactoring-compilation-engine

[github-actions]

Bug: message is never reported — missing .add().

Every other newError(...)/newInfo(...) call in this file ends with .add(), but here the built MessageBuilder is discarded, so the out-of-memory error is silently dropped. In the pre-refactor code this was addMessage(callback, CompilerMessageCategory.ERROR, ...), which did report it.

Suggested change

	callback.newError(CompilerLocalize.errorJavacOutOfMemory());
	callback.newError(CompilerLocalize.errorJavacOutOfMemory()).add();

[github-actions]

Bug: compiler error is swallowed — missing .add().

When wrapper.compile(...) throws a CompilerException, the error is built but never added to the context, so the failure is only logged to idea.log and the user sees a "successful" compile with no error message. Previously this path called context.addMessage(CompilerMessageCategory.ERROR, ...).

Suggested change

	context.newError(LocalizeValue.of(ExceptionUtil.getThrowableText(e)));
	context.newError(LocalizeValue.of(ExceptionUtil.getThrowableText(e))).add();

[github-actions]

Bug: infinite recursion / StackOverflowError. url(...) is the wrapper's own method, not the delegate's. On a successful lookup it re-invokes itself with the resolved VirtualFile.getUrl(), which resolves to the same file again and recurses forever (the message is also never forwarded to the delegate). This should delegate instead:

Suggested change

	return url(fileByURL.getUrl());
	return myDelegate.url(fileByURL.getUrl());

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 4m 2s —— View job

---

PR Review (re-review): Refactoring compilation engine

• Gather context (diff vs origin/master, changed files)
• Verify the migration bugs flagged in the previous review
• Re-scan all changed files for the lazy-API (new…() without .add()) pattern
• Post findings

✅ All three previously-flagged bugs are now fixed (commit e472cfd32 "Fixes.")

#	File · line	Status
1	JavaToolMonitor.java:191	Fixed — now return myDelegate.url(fileByURL.getUrl());, no more self-recursion / StackOverflowError.
2	JavaCompiler.java:111	Fixed — CompilerException path now ends with .add(), so the error is reported.
3	JavacOutputParser.java:163	Fixed — the OutOfMemoryError branch now ends with .add().

I re-scanned every new{Error,Warning,Info,Message}(…) call across all 11 changed files: each builder chain is now terminated with .add(). No remaining dropped-message regressions from the fluent-API migration.

Minor note (non-blocking)

• OutputParser.java (last line, } at line 96) still has no trailing newline at end of file — the sibling JavacOutputParser.java was given one in this PR, so it's worth matching for consistency. One-line fix: add a final newline.
  • _{(I tried to attach this as a one-click inline suggestion, but the CI shell guard blocks the newline-escaping needed to post a suggestion block via gh api; flagging here instead.)}

Verdict

The refactor is a clean mechanical reformat (tabs → 4-space, final/diamond cleanup) plus a consistent migration to the fluent MessageBuilder API, and the lazy-API hazard that caused bugs #2/#3 has been swept across the changed files. With the three fixes landed, this looks good to merge — the only outstanding item is the trivial trailing-newline nit above.
· branch refactoring-compilation-engine