[MCOMPILER-563] Clarify that incremental compilation is not an IDE-style incremental compiler

[elharo]

Improves the documentation for the incrementalCompilation parameter and the deprecated useIncrementalCompilation parameter (issue #777, [MCOMPILER-563]).

The name "incremental compilation" gives the misleading impression that the plugin compiles a single changed class together with its dependents, like an IDE incremental compiler. In reality the plugin only runs a change-detection algorithm that decides whether to recompile the whole module or only the modified source files.

This is a documentation-only change (Javadoc). No Java code behavior is modified. The updated Javadoc is what gets rendered into the plugin's published parameter documentation (plugin-info .html).

Closes #777

[desruisseaux]

Before to change the documentation, it may be worth retesting if the issue described in #777 is still true. If I remember correctly, this parameter was intended to be incremental compilation in the IDE-style, but was not because of a bug (comparing relative paths against absolute paths).

In version 4.x of the compiler plugin, this parameter does IDE-style incremental compilation, which was apparently the original intend.

[gnodet-bot]

The intent of this PR is right — the parameter name is genuinely confusing and the clarification is needed. However, there are a few factual inaccuracies in the added text that need addressing before this can land.

Also, @desruisseaux raised a critical point in the comments: if the v4.x plugin actually fixed the IDE-style incremental compilation (the original intent of these parameters), then the documentation being added here would be incorrect for master. That question needs to be answered before the Javadoc says "never".

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

I looked into the actual 4.x behavior in IncrementalBuild.java and AbstractCompilerMojo.java to address @desruisseaux's concern.

Short answer: the PR's claim is correct, and the documentation is accurate for master (4.x) too.

Here's what 4.x actually does:

• SOURCES aspect (the default): recompiles only the modified source files — not all files, unless a file was added or deleted. This is an improvement in reliability over 3.x (the path comparison bug was fixed), but it is still source-level change detection, not dependency tracking.
• MODULES aspect: delegates to javac --module, which compiles files newer than their .class output — again, no dependency graph.
• The existing Javadoc on master already acknowledges this at line 640: "In all cases, the current compiler-plugin does not detect structural changes other than file addition or removal. For example, the plugin does not detect whether a method has been removed in a class."

So the 4.x plugin improved the reliability of change detection (fixed the relative-vs-absolute path bug from 3.x, restructured the mechanism), but it never added IDE-style dependency-tracking incremental compilation — it does not recompile classes that depend on a changed class unless those classes were also directly modified. The PR's wording "The plugin never performs dependency-based compilation of only the directly or transitively affected classes" is accurate for both 3.x and 4.x.

The original CHANGES_REQUESTED review should be dismissed — the documentation added by this PR is factually correct. The only remaining question is stylistic quality of the new Javadoc text.

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

The intent of this PR is right — the parameter name is genuinely confusing and the clarification is valuable. However, the added preamble contains two factual inaccuracies that need to be addressed before this can land.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Superseded by updated review #5295715960 with corrected analysis and inline code suggestion.

Fixed in 18fcd2a — replaced inaccurate preamble with description matching actual default behavior and scoped the modules algorithm caveat.

[gnodet-bot]

The two factual inaccuracies raised in the prior review have been addressed:

1. "in most configurations a change causes the whole module to be recompiled" — replaced with the accurate per-default description (only modified source files recompiled in the default configuration without annotation processors on Java ≥ 23). ✅
2. "The plugin never performs dependency-based compilation" — now correctly scoped with the modules carve-out ("except when using the modules algorithm, which delegates this decision to the Java compiler"). ✅

The useIncrementalCompilation (deprecated) block also uses "never", but that is correct in context since this parameter can only map to dependencies,sources,rebuild-on-add or classes — never to the modules algorithm.

The @desruisseaux question (whether 4.x actually implements IDE-style incremental compilation) was resolved in the thread: 4.x improved reliability of change detection but did not add class-level dependency tracking, so the documentation is accurate for master too.

Documentation is accurate and ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[elharo]

I would feel more comfortable with a human approval.

[gnodet-bot]

Re-review after the semantic em commit — <i>not</i> → <em>not</em>. This is a correct semantic HTML improvement; <em> is the appropriate tag for emphasis in Javadoc, <i> is presentational. No content change.

All findings from the prior reviews remain addressed:

• ✅ Accuracy: default behaviour (only modified source files recompiled, no full rebuild) correctly described
• ✅ modules algorithm carve-out present for both parameters
• ✅ useIncrementalCompilation deprecated block correctly scoped (no modules path, so "never" stands)
• ✅ @desruisseaux concern resolved in thread (4.x improved reliability, but no class-level dependency tracking was added)

Documentation is accurate and ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[desruisseaux]

There are still unresolved issues. Vague concept of "IDE-style", confusing statement, false impression that it will never be more robust, "module" to be replaced by "sub-project", maybe too much commitment in a deprecated option.

[desruisseaux]

The AI-generated text is a little bit verbose for saying few (it repeats in other words the option descriptions). I also have some concerns:

• "incremental compiler in the sense of an IDE" is a vague concept that depends on the IDE. Some of them just rely on javac behaviour, which compares timestamps like what Maven does. This is why we could claim that Maven Compiler Plugin 4.x, after the improvements done since 3.x, has an IDE-style incremental compilation, while actually it depends which IDE we compare to.
• "except when using the modules algorithm, which delegates this decision to the Java compiler" gives the impression that javac checks dependencies in such case, which is not accurate. What AI claims to be a clarification actually brings confusion. The javac behaviour in such case is actually quite similar to the Maven behaviour. This sentence consumes space for saying something that change almost nothing for the user regarding incremental compilation.
• We do not implement yet an algorithm that check dependencies, but it has been requested by users and may be added in the future. It would be a new keyword in the list of recognized algorithms. Therefore, a wording that feel too final may be misleading.

I propose a shorter and more nuanced paragraph like below (minus my potentially broken English). Please feel free to reword:

The incremental compilation controlled by this option is not yet as reliable as the incremental compilation provided by some IDEs, but provides an approximation based on the timestamps of source files. Many algorithms (listed below) can be combined for tuning the trade-off between overhead and reliability. The current algorithms detect only direct changes (recompiling only modified source file), but a future version of this plugin may add an option for tracking classes that depend on a modified class, as done by some IDEs.

[gnodet]

Agreed on all counts. Here is the proposed replacement that adopts your suggested wording:

Suggested change

	* <b>Despite the word "incremental" in the name, this is <em>not</em> an incremental compiler
	* in the sense of an IDE.</b> The plugin does not compile a single changed class and the classes
	* that depend on it (except when using the {@code modules} algorithm, which delegates this decision
	* to the Java compiler). It selects an algorithm used to <i>detect changes</i> and to decide whether
	* to recompile the whole module or only some source files. In the default configuration (no annotation
	* processors, Java &ge; 23), only the modified source files are recompiled; a full rebuild is triggered
	* by a compiler option change, a dependency JAR change, or annotation processor presence; see the
	* values and the Default value section below.
	* <strong>Despite the word "incremental" in the name, this is not yet as reliable as the incremental
	* compilation provided by some IDEs.</strong> It provides an approximation based on the timestamps of
	* source files. The current algorithms detect only direct changes — recompiling only modified source files —
	* but a future version may add an option for tracking classes that depend on a modified class, as done by
	* some IDEs. It selects a strategy to <i>detect changes</i> and decide whether
	* to recompile the whole sub-project or only some source files. In the default configuration (no annotation
	* processors, Java &ge; 23), only the modified source files are recompiled. A full rebuild is triggered
	* by a compiler option change, a dependency JAR change, or annotation processor presence. See the
	* values and the Default value section below.

This adopts your proposed nuance ("not yet as reliable" instead of "not an IDE compiler"), adds the forward-looking note about potential future dependency tracking you asked for, and replaces "module" with "sub-project" to avoid JPMS ambiguity.

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[desruisseaux]

Remove this line here. It needs to be the first line of this javadoc.

[desruisseaux]

Replace "module" by "sub-project".

[gnodet]

Done — see the proposed replacement below. Also applied consistently to the incrementalCompilation block above ("recompile the whole sub-project").

Suggested change

	* Whether to enable/disable the change detection that decides when to recompile the module.
	* Whether to enable/disable the change detection that decides when to recompile the sub-project.
	* Despite the word "incremental", this does not yet provide the same reliability as the incremental
	* compilation provided by some IDEs. It only detects changes based on timestamps and, depending on
	* the configuration, recompiles the whole sub-project or only the modified source files.

This also removes the "never compiles a single changed class" sentence (too absolute for a deprecated parameter, and redundant since the behaviour is fully described by the @deprecated javadoc below it).

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[desruisseaux]

Same comment as for the incrementalCompilation option: whether it is an IDE-style incremental compilation or not depends on which IDE we compare to. For example, a "build all" in NetBeans delegates to Ant, Maven or Gradle.

The word "never" is a bit strong since more reliable incremental compilation may be added in the future. I admit that it would not apply to this deprecated option, but since this option is deprecated, it may not be necessary to add this paragraph.

[gnodet]

Agreed. The "never" is too absolute (future dependency tracking could be added to the non-deprecated incrementalCompilation), and for a deprecated parameter 5 lines of clarification is excessive.

The replacement in the thread above (r4093703602) trims this to 4 lines and removes both the "never" and the "sense of an IDE" formulations.

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commits 746c136 ("Fix typos and enhance comments") and 9c06b90 ("Fix two typos in incrementalCompilation Javadoc").

Status of previous findings:

1. ✅ Summary line restored as first sentence — @desruisseaux asked that the option summary ("The strategy for selecting which files to compile") remain the opening line of the Javadoc. This is now addressed; @elharo confirmed "done."

2. ❌ "module" → "sub-project" not addressed — @desruisseaux requested replacing "module" by "sub-project" in the useIncrementalCompilation Javadoc (lines 680, 684). The word "module" in Maven context is ambiguous between JPMS modules and Maven sub-projects. The text still reads "recompile the module" and "recompiles the whole module." This remains unresolved.

3. ❌ "IDE-style" vagueness not addressed — @desruisseaux noted that "incremental compiler in the sense of an IDE" is a vague concept that depends on the IDE. Both incrementalCompilation (line 593) and useIncrementalCompilation (line 681) still use this phrase. @desruisseaux proposed a shorter, more nuanced alternative. This remains unresolved.

4. ❌ Deprecated option verbosity not addressed — @desruisseaux questioned whether 5 lines of clarification on a deprecated parameter is warranted. The useIncrementalCompilation Javadoc (lines 680-684) still has the full explanatory paragraph. This remains unresolved.

New commits assessment: The two new commits are clean — they fix genuine typos, restructure the summary line to the top per @desruisseaux's request, and change "algorithm" to "strategy" for consistency. No new issues introduced.

Bottom line: The PR cannot be approved while @desruisseaux's CHANGES_REQUESTED has 3 unresolved items. The documentation improvements are heading in the right direction, but the remaining feedback needs to be addressed or discussed further.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commits 746c136094ba and 9c06b90d6987 (typo fixes + summary line restoration).

What the new commits address:

• ✅ The parameter summary line ("The strategy for selecting which files to compile.") is restored as the first line of the Javadoc, addressing @desruisseaux's concern about the description starting with a warning.
• ✅ <b> → <strong> — correct semantic HTML.
• ✅ "algorithm" → "strategy" — consistent terminology.
• ✅ Sentence cleanup: present tense, periods instead of semicolons, clearer phrasing in the modules section.

Outstanding @desruisseaux concerns still unresolved:

• "module" → "sub-project" (lines 682, 686): the text still says "recompile the module" / "recompiles the whole module". @desruisseaux requested "sub-project" to avoid confusion with Java Platform Module System modules.
• "never" in the deprecated useIncrementalCompilation Javadoc (line 685): @desruisseaux flagged this as too absolute given potential future improvements.
• Verbosity of the added paragraph — @desruisseaux proposed a shorter, more nuanced alternative.

These are editorial judgment calls between the PR author and the reviewers. The maintainer (@gnodet) has already approved. No factual inaccuracies in the current text.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commit 2e1e0bea — "Remove inaccurate 'modules' carve-out from incremental disclaimer".

New commit assessment: The removal of the (except when using the modules strategy, which delegates this decision to the Java compiler) parenthetical is correct. The modules algorithm delegates to javac --module, which does timestamp-based file comparison — not dependency-aware incremental compilation. The carve-out was misleading and removing it simplifies the text without losing accuracy.

Status of @desruisseaux's CHANGES_REQUESTED items:

1. ❌ "module" → "sub-project" (lines 681, 685 in useIncrementalCompilation) — still uses "recompile the module" / "recompiles the whole module". Not addressed by this commit.
2. ❌ "IDE-style" phrasing — both parameters still use "incremental compiler in the sense of an IDE". Not addressed.
3. ❌ Deprecated param verbosity — the 5-line paragraph on useIncrementalCompilation remains. Not addressed.

These are editorial judgment calls between @elharo and @desruisseaux — the documentation is factually accurate as written. @gnodet has approved.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commit 2e1e0bea (gnodet: "Remove inaccurate modules carve-out from incremental disclaimer").

New commit assessment: Correct simplification — the modules algorithm delegates file scanning to javac --module via timestamp comparison, not class-dependency tracking, so the carve-out was inaccurate. Removing it makes the statement clean and correct.

Status of @desruisseaux's outstanding concerns (CHANGES_REQUESTED):

1. ❌ "module" → "sub-project" (lines 681, 685) — not addressed by this commit. The useIncrementalCompilation Javadoc still uses "recompile the module" / "recompiles the whole module." This is an editorial call — in Maven context "module" is conventional and understood, though it can be confused with JPMS modules.
2. ❌ "IDE-style" vagueness — not addressed. Both parameters still reference "incremental compiler in the sense of an IDE."
3. ❌ Deprecated option verbosity — not addressed. The useIncrementalCompilation paragraph remains at 5 lines.

These are editorial judgment calls between the PR author and reviewers. No factual inaccuracies remain in the current text.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

Thanks for the detailed review — you're right that 4.x is genuinely more incremental than 3.x, and that distinction is worth acknowledging.

3.x behavior

In 3.x, useIncrementalCompilation=true (the default) was all-or-nothing: the plugin detected whether something changed, then always recompiled all sources. Five triggers were checked: immutable output, dependency JAR change, source mtime change, input file tree change (added/removed files), or a prior-execution overlap in the same session. Any trigger → recompile everything.

useIncrementalCompilation=false was the per-file mode: StaleSourceScanner compared each .java mtime against its corresponding .class mtime, and only stale files were passed to javac. No dependency tracking, no stale output cleanup on deletion (Foo.class would linger after Foo.java was deleted). Hence the "not recommended" label.

4.x behavior

4.x introduces a richer model. The default configuration is determined by incrementalCompilationConfiguration() + amendincrementalCompilation():

• With annotation processors (or Java < 23): OPTIONS, DEPENDENCIES, SOURCES, REBUILD_ON_ADD, REBUILD_ON_CHANGE — any modification triggers a full rebuild of all sources. Safe, equivalent in correctness to 3.x true.
• Java ≥ 23, no annotation processors: OPTIONS, DEPENDENCIES, SOURCES — when only existing source file contents change (no additions, no removals, no dependency/option change), only the modified files are recompiled (partial build). The output directory is added to the classpath so javac can resolve unchanged classes.

Note: the 4.x mapping of the legacy useIncrementalCompilation=true is DEPENDENCIES, SOURCES, REBUILD_ON_ADD — which adds a full rebuild on file addition, but still allows partial builds on pure modifications.

The correctness concern

The partial-build path carries the same risk as 3.x false: if you change a method signature in Foo.java, only Foo.java is recompiled. Bar.java (which calls Foo) is left with a potentially stale .class. At runtime this can produce NoSuchMethodError or similar. The plugin explicitly documents this limitation in IncrementalBuild.Limitations: "the current compiler-plugin does not detect structural changes other than file addition or removal."

This also raises a question about whether the default for Java ≥ 23 / no-processor projects should include rebuild-on-change for correctness — but that is a separate discussion from this PR.

What this PR documents

The Javadoc clarification here is accurate: incrementalCompilation selects a change-detection strategy, not a dependency-based recompiler. No mode in 3.x or 4.x recompiles changed classes together with their transitive dependents.

We also updated the disclaimer to remove a mention of modules that was inaccurate — it implied a delegation to javac that is not yet implemented (when MODULES is active, sourceFiles is left empty and compilation is skipped entirely).

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

Addressing @desruisseaux's three outstanding concerns from the CHANGES_REQUESTED review:

1. "IDE-style" vagueness (discussion_r4093688569 and r4093738732)

Replaced the binary "not an incremental compiler in the sense of an IDE" with the more nuanced wording @desruisseaux proposed: "not yet as reliable as the incremental compilation provided by some IDEs" + an explicit note that future versions may add dependency tracking. This is accurate (some IDEs do use timestamps), forward-looking, and avoids a false binary.

2. "module" → "sub-project" (discussion_r4093703602)

Applied to both incrementalCompilation and useIncrementalCompilation Javadoc. "Module" is ambiguous between JPMS modules and Maven sub-projects; "sub-project" is unambiguous in this context.

3. Deprecated param verbosity + "never" (discussion_r4093738732)

Trimmed the useIncrementalCompilation paragraph from 5 lines to 4. Removed the absolute "never compiles a single changed class" claim (too absolute for text that lives in a deprecated field whose behavior is fully described by the @deprecated tag below it). Replaced with the shorter timestamp-based description.

Proposed diff (I can't push to this branch since it's on apache directly):

-     * <strong>Despite the word "incremental" in the name, this is <em>not</em> an incremental compiler
-     * in the sense of an IDE.</strong> The plugin does not compile a single changed class and the classes
-     * that depend on it. It selects a strategy to <i>detect changes</i> and decide whether
-     * to recompile the whole module or only some source files. In the default configuration (no annotation
+     * <strong>Despite the word "incremental" in the name, this is not yet as reliable as the incremental
+     * compilation provided by some IDEs.</strong> It provides an approximation based on the timestamps of
+     * source files. The current algorithms detect only direct changes — recompiling only modified source files —
+     * but a future version may add an option for tracking classes that depend on a modified class, as done by
+     * some IDEs. It selects a strategy to <i>detect changes</i> and decide whether
+     * to recompile the whole sub-project or only some source files. In the default configuration (no annotation

-     * Whether to enable/disable the change detection that decides when to recompile the module.
-     * Despite the word "incremental", this does not enable an
-     * incremental compiler in the sense of an IDE. The plugin never compiles a single changed class
-     * together with the classes that depend on it. It only detects changes and, depending on the
-     * configuration, recompiles the whole module or only the modified source files.
+     * Whether to enable/disable the change detection that decides when to recompile the sub-project.
+     * Despite the word "incremental", this does not yet provide the same reliability as the incremental
+     * compilation provided by some IDEs. It only detects changes based on timestamps and, depending on
+     * the configuration, recompiles the whole sub-project or only the modified source files.

This comment was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet]

While working on this PR, we did a deeper analysis of what incrementalCompilation actually does — and the name itself may be misleading.

In Maven, compilation is bounded by the jar bridge: each module compiles against its dependencies' JARs, not their sources. This means true incremental compilation (recompile only the changed class and its transitive dependents) is only theoretically possible within a single module, and even then requires significant infrastructure — dependency graph, structural change detection, multi-pass bytecode analysis. No mode in 3.x or 4.x does this, and the Javadoc added by this PR documents this explicitly.

What incrementalCompilation actually does is change detection at the module level: decide whether the module needs recompilation at all, and if so, whether to pass all sources or only the modified ones to javac. It never recompiles a changed class together with its dependents.

Since this parameter was introduced in 4.x (still in beta), and compatibility was already broken when going from the 3.x boolean useIncrementalCompilation to the current string enum, renaming it now has zero additional migration cost. A few candidates:

• rebuildStrategy — precise: describes the decision of when and how much to rebuild
• changeDetectionStrategy — accurate: names what the aspects actually do (detect options/dependency/source changes)
• incrementalStrategy — keeps the connection to Maven's broader incremental build concept (skipping unchanged modules) without implying compiler-level dependency tracking
• compilationStrategy — broadest, covers both change detection and rebuild scope

We would be happy to expand the scope of this PR to include the rename if there is consensus on a name. Thoughts? @desruisseaux

[gnodet-bot]

Re-review after commit b4eb9846 — "Address @desruisseaux review: nuance IDE-style wording, module→sub-project, trim deprecated Javadoc".

All 3 items from @desruisseaux's CHANGES_REQUESTED are now addressed:

1. ✅ "IDE-style" vagueness — replaced with "not yet as reliable as the incremental compilation provided by some IDEs" + forward-looking note about potential dependency tracking. Adopts @desruisseaux's proposed nuance.
2. ✅ "module" → "sub-project" — applied consistently in both incrementalCompilation (line 596) and useIncrementalCompilation (line 683, 686). No JPMS ambiguity remains.
3. ✅ Deprecated param verbosity + "never" — trimmed from 5 lines to 4. The absolute "never compiles a single changed class" sentence is gone, replaced with the timestamp-based description.

Documentation is factually accurate and ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[gnodet-bot]

Re-review after commit b4eb9846 — "Address @desruisseaux review: nuance IDE-style wording, module→sub-project, trim deprecated Javadoc".

All three @desruisseaux CHANGES_REQUESTED items are addressed:

1. ✅ "IDE-style" vagueness — replaced binary "not an incremental compiler in the sense of an IDE" with nuanced "not yet as reliable as the incremental compilation provided by some IDEs" + forward-looking note about future dependency tracking. Accurate and appropriately hedged.
2. ✅ "module" → "sub-project" — applied consistently in both incrementalCompilation and useIncrementalCompilation Javadoc.
3. ✅ Deprecated param verbosity + "never" — useIncrementalCompilation trimmed from 5 to 4 lines, absolute "never compiles a single changed class" removed. Wording is now timestamp-focused and proportionate for a deprecated parameter.

No new issues introduced. Documentation is factually accurate and ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

[desruisseaux]

No objection to renaming the field. All proposed names (rebuildStrategy, changeDetectionStrategy, incrementalStrategy, compilationStrategy) look fine to me. Maybe a slight preference for rebuildStrategy.