Skip to content

Support external Eclipse formatter profile via -Asimplebuilder.formatterProfile (#278) - #284

Merged
AndreasIgel merged 25 commits into
java-helpers:mainfrom
AndreasIgel:devin/1788610730-formatter-profile-278
Sep 20, 2026
Merged

AndreasIgel merged 25 commits into
java-helpers:mainfrom
AndreasIgel:devin/1788610730-formatter-profile-278

Conversation

@AndreasIgel

Copy link
Copy Markdown
Collaborator

Summary

Resolves #278: the JDT formatter profile used for generated builders can now be replaced per project instead of being hard-wired to the bundled eclipse-java-format.xml.

New project-wide compiler option (no annotation override, as scoped in the issue):

-Asimplebuilder.formatterProfile=<file path | classpath resource>

Resolution in RoasterSourceFormatter.loadFormatterProperties():

if no custom profile configured      -> loadBundledProfile()
else loadConfiguredProfile(location)  // regular file first, then classpath resource; warns "…falling back to the bundled profile"
       .orElseGet(loadBundledProfile) // warns "…not found on the processor classpath"; empty Properties
                                      //   -> existing "JDT requested but profile unavailable" lightweight fallback

Both are built on a shared readProfile(location); the path is used as given (no placeholder substitution — ${project.basedir} in the Maven example is interpolated by Maven before javac runs).

Plumbing: CompilerArgumentsEnum.FORMATTER_PROFILEProcessingContext.getFormatterProfile() (trimmed, null = default) → RoasterCodeGenerator(ProcessingContext) (now the single constructor; the context gained getProcessingEnvironment()/getLogger()) → RoasterSourceFormatter(logger, mode, formatterProfile) (3-arg ctor now public, blank → default).

Behavior change to note: a missing custom profile previously (via the package-private test ctor) degraded to lightweight formatting; now it falls back to the bundled profile with a warning, so JDT formatting stays on. Two RoasterSourceFormatterTest cases asserting the old lightweight degradation were replaced by constructor_missingProfile_fallsBackToBundledProfile; the "profile unavailable → lightweight" path now only triggers when the bundled resource itself is absent.

Tests: filesystem profile (tab-indent variant of the bundled XML, asserts tabs in output vs spaces for default), explicit classpath name, missing path, blank value, malformed XML, CompilerArgumentsReader read, and an end-to-end compilation with -Asimplebuilder.formatterProfile=<tempfile> in ConfigurationProcessingTest. Documented under formattingMode in docs/CONFIGURATION.md.

Full processor suite: 423 tests green; example module regenerates byte-identically.

AndreasIgel and others added 17 commits August 15, 2026 10:57
The class-level Javadoc and docs/CONFIGURATION.md implied that
@SimpleBuilder is inherited by subclasses, but the annotation was not
meta-annotated with @inherited. As a result BuilderProcessor, which
collects types via RoundEnvironment.getElementsAnnotatedWith(...),
only produced builders for the exact type carrying @SimpleBuilder and
not for unannotated subclasses.

Add @inherited to @SimpleBuilder so subclasses are treated as if they
also carried the annotation, mirroring the existing behaviour of
@SimpleBuilder.Template (which is already @inherited). Update the
Javadoc to document the inheritance explicitly and clarify the
CONFIGURATION.md wording. @Ignore4BuilderGeneration still suppresses
generation for the exact type it is placed on, so opt-outs continue
to work as before.

Add SimpleBuilderInheritanceTest covering direct inheritance, the
opt-out interaction, and multi-level (grandchild) inheritance.

Closes java-helpers#244

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ance

Rename SimpleBuilderInheritanceTest to BuilderAnnotationInheritanceTest
so the name reflects that it covers both builder-triggering annotations.
Add unannotatedSubclassGetsBuilderFromInheritedTemplate, which verifies
that a custom @inherited template annotation (meta-annotated with
@SimpleBuilder.Template) propagates to unannotated subclasses, matching
the existing behaviour of @SimpleBuilder itself.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
The Template Javadoc and CONFIGURATION.md "Template Annotations" section
did not explain that @SimpleBuilder.Template is @inherited, nor that a
custom template annotation must additionally declare @inherited to
propagate to unannotated subclasses. Add explicit documentation and an
example showing the @inherited custom annotation pattern.

Also move assertNoBuilderGenerated to ProcessorAsserts so it is shared
by BuilderAnnotationInheritanceTest and Ignore4BuilderGenerationTest
instead of being duplicated as a private helper in each test class.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
While @SimpleBuilder and @inherited template annotations now correctly
trigger builder generation for unannotated subclasses, the configuration
options declared on the parent's @SimpleBuilder(options = ...) or template
are not yet applied to inherited subclass builders — they use default
options instead. This is tracked separately in issue java-helpers#245.

Add caveats to the SimpleBuilder Javadoc, the CONFIGURATION.md Template
Annotations section, and the Template Annotations Not Working
troubleshooting section so users are not surprised by this limitation.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Add explicit guidance that @SimpleBuilder.Template is a meta-annotation
for custom annotation declarations (@interface) only and cannot be
placed directly on a class or record. @SimpleBuilder is for direct
one-off annotation of classes/records.

- SimpleBuilder.java: add 'When to use' section to class-level Javadoc
- SimpleBuilder.Template Javadoc: state it can only be placed on
  annotation types (ANNOTATION_TYPE), not on classes/records
- CONFIGURATION.md 'Template Annotations': add comparison table and
  introductory paragraph
- CONFIGURATION.md troubleshooting: add item about @SimpleBuilder.Template
  not being a class annotation

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…Javadoc

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…arget

The compiler and IDE already enforce @target(ANNOTATION_TYPE) and show
a clear error when @SimpleBuilder.Template is placed on a class/record,
so this troubleshooting item adds no value.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…formatterProfile (java-helpers#278)

Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
Co-Authored-By: Andreas Igel <andreas.igel@computacenter.com>
@codecov

codecov Bot commented Sep 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Kept both sides' new tests: formatter profile test alongside upstream's
system-property fallback tests and the buildFullyConfigured() refactor.
…rmatter-profile-278

# Conflicts:
#	processor/src/main/java/org/javahelpers/simple/builders/processor/processing/ProcessingContext.java
#	processor/src/test/java/org/javahelpers/simple/builders/processor/ConfigurationProcessingTest.java
#	processor/src/test/java/org/javahelpers/simple/builders/processor/classgen/roaster/RoasterSourceFormatterTest.java
AndreasIgel and others added 3 commits September 20, 2026 00:41
Keep formatter dependencies explicit, log the effective formatting strategy,
and centralize profile customization in focused test helpers.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Let formatter tests request configured profile files directly from the shared
helper instead of duplicating temporary-file setup.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment on lines +115 to +117
ProcessingEnvironment processingEnv,
ProcessingLogger logger,
PerformanceTracker tracker,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

All of them is available in context right? But just replacing 3 parameters by context is not enough. Context should replace the member variables here too. Idee is to have less getter and not to add getlogger or other new getters to context

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

RoasterCodeGenerator now stores only ProcessingContext. Logging, performance phases, source-file creation, type lookup, and formatter creation go through narrow context operations; no raw logger/environment/profile getters were added.

Comment thread docs/CONFIGURATION.md
"simple-builders: Bundled Eclipse formatter profile '%s' was not found on the processor classpath.",
formatterProfileResource);
return new Properties();
"simple-builders: Eclipse formatter profile '%s' was not found as a file or classpath resource; falling back to the bundled profile.",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Here is still a simple builder prefix. Do we need this? Is it not logged below execution of simple builder and does this warning not lead to an abort of code generation?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Removed the redundant simple-builders: prefix from formatter warnings. The warning does not abort generation: it explicitly reports the fallback, first to the bundled profile and ultimately to lightweight formatting if no profile is available.

formatterProfileResource,
StringUtils.defaultIfBlank(ex.getMessage(), ex.getClass().getSimpleName()));
return new Properties();
"simple-builders: Failed to load Eclipse formatter profile '%s': %s; falling back to the bundled profile.",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Here again with simple-builder prefix. Do we do that always on warnings? And if this is the case, should it not be part of warning function instead of being repeated?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Removed the formatter-specific prefixes for consistency with other ProcessingLogger warning callers. I did not move a prefix into ProcessingLogger because that would change all warnings globally and existing callers currently provide plain contextual messages.

return properties.get();
}
logger.warning(
"simple-builders: Bundled Eclipse formatter profile '%s' was not found on the processor classpath.",

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

More precisely, it has not been found in the configuration file of class path!?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Made the bundled case more precise: it now says Bundled Eclipse formatter profile resource ... was not found on the processor classpath. The configured-profile message still distinguishes lookup as a filesystem path or classpath resource.

Comment on lines +495 to +497
private String describe(Exception ex) {
return StringUtils.defaultIfBlank(ex.getMessage(), ex.getClass().getSimpleName());
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Is it usually to have an exception without message? And if this is the case, what about the cause or more text then just the class name. And function should be renamed from "describe"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Renamed describe to getExceptionDetails. It now always includes the exception class, includes a nonblank message when present, and appends distinct root-cause class/message details. Message-less exceptions therefore still produce useful output.

"JDT mode with missing profile should fall back to lightweight (tabs converted)");
String defaultResult = defaultFormatter.format(input);
assertTrue(result.contains("\t"), "Custom tab profile should produce tab indentation");
assertTrue(!defaultResult.contains("\t"), "Bundled profile should produce spaces");

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Would nit be assertFalse be better here and nit using "!"? Or even better is there no assert for not containing?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Changed this and the other equivalent checks to assertFalse(...contains(...)) for direct readability.

Comment on lines +645 to +651
assertTrue(
capturing.messages().stream()
.anyMatch(
w ->
w.contains("Failed to load")
&& w.contains("eclipse-java-format-malformed.xml")),
w.startsWith(
"WARNING: simple-builders: Failed to load Eclipse formatter profile 'eclipse-java-format-malformed.xml': ")
&& w.endsWith("; falling back to the bundled profile.")),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Could we have a helper method for this kind of assert or do we not already have one?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Added CapturingProcessingLogger.assertMessageMatching(Predicate<String>, String) and use it here. The helper provides the expected description plus all captured messages on failure.

AndreasIgel and others added 2 commits September 20, 2026 16:00
Let ProcessingContext own code-generation infrastructure, simplify formatter
mode initialization, and improve fallback diagnostics and focused assertions.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Keep source output owned by the code generator, reuse shared processing test
infrastructure, and rely on Throwable descriptions for fallback diagnostics.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@sonarqubecloud

Copy link
Copy Markdown

@AndreasIgel
AndreasIgel merged commit f0bb51a into java-helpers:main Sep 20, 2026
6 checks passed
@AndreasIgel
AndreasIgel deleted the devin/1788610730-formatter-profile-278 branch September 20, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support external Eclipse formatter profile via compiler argument

2 participants