Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
76c8e02
fix(rename-class): improve conflict and rename log readability
rawvoid Aug 2, 2026
52b072b
fix(element-wrapper): align logging with rename-class style
rawvoid Aug 2, 2026
cc5f575
refactor(rename-class): prefer formatted templates in conflict messages
rawvoid Aug 2, 2026
b58a684
fix(element-wrapper): log removed wrapper names at DEBUG
rawvoid Aug 2, 2026
2b9a052
fix(dedupe-class): align logging with rename/element-wrapper style
rawvoid Aug 2, 2026
68d9624
fix(promote-nested-class): align logging with rename/element-wrapper …
rawvoid Aug 2, 2026
f70780f
fix(remove-unused-class): align logging with rename/element-wrapper s…
rawvoid Aug 2, 2026
1df365b
fix(flatten-multi-element-prop): align logging with rename/element-wr…
rawvoid Aug 2, 2026
44f2909
Merge pull request #192 from rawvoid/fix/rename-class-logging
rawvoid Aug 2, 2026
ca9f5cb
Merge pull request #193 from rawvoid/fix/element-wrapper-logging
rawvoid Aug 2, 2026
67b5ae4
Merge pull request #194 from rawvoid/fix/dedupe-class-logging
rawvoid Aug 2, 2026
9be78c2
Merge pull request #195 from rawvoid/fix/promote-nested-class-logging
rawvoid Aug 2, 2026
5f24166
Merge pull request #196 from rawvoid/fix/remove-unused-class-logging
rawvoid Aug 2, 2026
099ca29
Merge pull request #197 from rawvoid/fix/flatten-multi-element-prop-l…
rawvoid Aug 2, 2026
af9dcb9
fix(plugins): address logging review findings
rawvoid Aug 2, 2026
78571fd
fix(rename-multi-element-prop): align logging style
rawvoid Aug 2, 2026
bc47e93
Merge pull request #198 from rawvoid/fix/logging-review-followups
rawvoid Aug 2, 2026
30bbe92
Merge pull request #199 from rawvoid/fix/rename-multi-element-prop-lo…
rawvoid Aug 2, 2026
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
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@
* Shell-to-shell exact merges remain allowed. Default is <strong>auto</strong>: on when
* {@code -Xelement-wrapper} is also active; force with {@code true}/{@code false}.
* </p>
* <p>
* Logging: each accepted merge is {@code DEBUG}; merge and element-class-clear counts are
* {@code INFO}. Routine skips stay {@code DEBUG}; element-class root mismatches and
* ObjectFactory collisions after dedupe are {@code WARN} (ObjectFactory: one multi-line
* summary).
* </p>
*
* @author Rawvoid
*/
Expand Down Expand Up @@ -869,7 +875,11 @@ public void postProcessModel(Model model, ErrorHandler errorHandler) {
warnObjectFactoryCollisions(model);
}
if (merged > 0) {
log.info("Deduped {} bean merge(s){}", merged, session.dry ? " (dry-run)" : "");
if (session.dry) {
log.info("Deduped {} bean merge(s) (dry-run)", merged);
} else {
log.info("Deduped {} bean merge(s)", merged);
}
}
}

Expand Down Expand Up @@ -1019,7 +1029,7 @@ private boolean tryMerge(

if (!isPackageLevel(host) && host.parent() != victim.parent() && !allowCrossNestedParent) {
log.debug(
"Skip dedupe {}: cross-hierarchy nested '{}' vs '{}'",
"Skip dedupe {}: cross-hierarchy nested {} vs {}",
reason, victim.fullName(), host.fullName()
);
return false;
Expand All @@ -1028,11 +1038,11 @@ private boolean tryMerge(
if (victim.isElement() && host.isElement()
&& !Objects.equals(victim.getElementName(), host.getElementName())) {
log.warn(
"Skip dedupe {}: both '{}' and '{}' are element-classes with different roots ({} vs {})",
"Skip dedupe {}: both element-classes with different roots: {} ({}) vs {} ({})",
reason,
victim.fullName(),
host.fullName(),
victim.getElementName(),
host.fullName(),
host.getElementName()
);
return false;
Expand All @@ -1044,7 +1054,7 @@ private boolean tryMerge(
&& ModelUtils.isPureCollectionShell(victim)
&& !ModelUtils.isPureCollectionShell(host)) {
log.debug(
"Skip dedupe {}: pure collection shell '{}' must not merge into non-shell '{}'",
"Skip dedupe {}: pure collection shell {} must not merge into non-shell {}",
reason, victim.fullName(), host.fullName()
);
return false;
Expand All @@ -1057,8 +1067,8 @@ private boolean tryMerge(
return false;
}

log.info(
"Dedupe {}: '{}' -> '{}' (nameKey={})",
log.debug(
"Dedupe {}: {} → {} (nameKey={})",
reason, victim.fullName(), host.fullName(), nameKey(victim.shortName)
);

Expand Down Expand Up @@ -1102,7 +1112,11 @@ private boolean prepareNestedMerges(Model model, Session session, CClassInfo vic
var childNorm = child.shortName.toLowerCase(Locale.ROOT);
for (var hc : directNestedBeans(model, host)) {
if (hc.shortName.toLowerCase(Locale.ROOT).equals(childNorm)) {
log.debug("Skip dedupe: nested name clash {} under {}", child.shortName, host.fullName());
log.debug(
"Skip dedupe: nested name clash {} under {}",
child.shortName,
host.fullName()
);
return false;
}
}
Expand Down Expand Up @@ -1165,9 +1179,10 @@ private boolean prepareNestedEnums(Model model, Session session, CClassInfo vict
}
}
if (match != null) {
log.info(
"Dedupe exact-enum: '{}' -> '{}'",
fullEnumName(victimEnum), fullEnumName(match)
log.debug(
"Dedupe exact-enum: {} → {}",
fullEnumName(victimEnum),
fullEnumName(match)
);
session.countedMerges.add(IdentityPair.directed(victimEnum, match));
if (!session.dry) {
Expand Down Expand Up @@ -1298,7 +1313,7 @@ private static void collapseRedundantElementClasses(Model model, Set<String> mer
if (mergedPackageNameKeys.isEmpty()) {
return;
}
var cleared = 0;
var cleared = new ArrayList<String>();
for (var elementInfo : model.getAllElements()) {
if (!elementInfo.hasClass()) {
continue;
Expand All @@ -1318,19 +1333,32 @@ private static void collapseRedundantElementClasses(Model model, Set<String> mer
&& !mergedPackageNameKeys.contains(pkg + '\0' + contentKey)) {
continue;
}
// Capture before clearing className: fullName() falls back to type.fullName()
// (JAXBElement<…>) once className is null.
cleared.add(elementInfo.fullName());
setFieldValue(CELEMENTINFO_CLASSNAME_FIELD, elementInfo, null);
cleared++;
}
if (cleared > 0) {
log.info("Cleared {} redundant element class name(s) after dedupe", cleared);
if (!cleared.isEmpty()) {
log.info("Cleared {} redundant element class name(s) after dedupe", cleared.size());
log.debug(
"Cleared element class name(s):\n {}",
String.join("\n ", cleared)
);
}
}

private static void warnObjectFactoryCollisions(Model model) {
var collisions = new ArrayList<String>();
for (var group : ModelUtils.objectFactorySqueezedCollisions(model)) {
var squeezed = group.getFirst().getSqueezedName();
var names = group.stream().map(CClassInfo::fullName).toList();
collisions.add("squeezed '%s': %s".formatted(squeezed, names));
}
if (!collisions.isEmpty()) {
log.warn(
"ObjectFactory squeezed-name collision after dedupe (package-local createXxx): {}",
group.stream().map(CClassInfo::fullName).toList()
"ObjectFactory name collision(s) after dedupe ({}):\n {}",
collisions.size(),
String.join("\n ", collisions)
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@
* rewrites that shape into a repeated element list with
* {@code @XmlElementWrapper(nillable = true)} and drops the synthetic local element info.
* </p>
* <p>
* Logging: each flattened field and the removed-wrapper name list are {@code DEBUG}; the
* flatten count and removed-wrapper count are {@code INFO}. Skipped wrapper removals and
* stale annotation owners (plugin-order issues) are one multi-line {@code WARN} each.
* Per-field flatten failures stay single-line {@code WARN}.
* </p>
*
* @author Rawvoid
*/
Expand Down Expand Up @@ -136,22 +142,25 @@

@Override
public boolean run(Outline outline, Options opt, ErrorHandler errorHandler) throws SAXException {
var skipped = new ArrayList<String>();
for (var flattened : flattenedFields) {
// Never use Outline#getClazz to test liveness: it lazily creates ClassOutline for
// beans already removed from the model and desynchronizes beans vs classes.
if (!outline.getModel().beans().containsValue(flattened.owner())) {
// Stale after a later merge (e.g. dedupe): the class is not generated. Isomorphic
// hosts already have their own FlattenedField from this plugin's model pass.
log.info(
"Skipping @XmlElementWrapper for removed flatten owner '{}' property '{}' "
+ "(prefer -Xelement-wrapper after other model-mutating plugins)",
flattened.owner().fullName(),
flattened.propertyName()
);
skipped.add("%s.%s".formatted(flattened.owner().fullName(), flattened.propertyName()));
continue;
}
annotateXmlElementWrapper(outline, flattened);
}
if (!skipped.isEmpty()) {
log.warn(
"Skipped {} @XmlElementWrapper annotation(s); owner removed (prefer -Xelement-wrapper after other model-mutating plugins):\n {}",
skipped.size(),
String.join("\n ", skipped)

Check failure on line 161 in plugins/src/main/java/io/github/rawvoid/jaxb/plugin/ElementWrapperPlugin.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of duplicating this literal " " 3 times.

See more on https://sonarcloud.io/project/issues?id=rawvoid_jaxb-plugins&issues=AZ_B8Ie9UItUTMcR3nVE&open=AZ_B8Ie9UItUTMcR3nVE&pullRequest=201
);
}
return true;
}

Expand Down Expand Up @@ -205,12 +214,19 @@

if (resolved.orphanElementInfo() != null
&& !removeElementInfo(model, resolved.orphanElementInfo())) {
log.warn("Could not remove synthetic element info for {}.{}",
owner.fullName(), outer.getName(false));
log.warn(
"Could not remove synthetic element info for {}.{}",
owner.fullName(),
outer.getName(false)
);
}

log.debug("Flattened {}.{} (wrapper {})",
owner.fullName(), outer.getName(false), resolved.wrapper().fullName());
log.debug(
"Flattened {}.{} (wrapper {})",
owner.fullName(),
outer.getName(false),
resolved.wrapper().fullName()
);
}
}

Expand Down Expand Up @@ -367,8 +383,11 @@
CElementPropertyInfo replacement
) {
if (replacement.ref().isEmpty()) {
log.warn("Skip flattening {}.{}: replacement has no type refs",
owner.fullName(), outer.getName(false));
log.warn(
"Skip flattening {}.{}: replacement has no type refs",
owner.fullName(),
outer.getName(false)
);
return false;
}

Expand All @@ -378,8 +397,11 @@
owner.addProperty(replacement);
if (properties.size() != sizeBefore + 1 || properties.getLast() != replacement) {
// addProperty no-op'd — model unchanged.
log.warn("Skip flattening {}.{}: addProperty did not append the replacement",
owner.fullName(), outer.getName(false));
log.warn(
"Skip flattening {}.{}: addProperty did not append the replacement",
owner.fullName(),
outer.getName(false)
);
return false;
}

Expand All @@ -402,21 +424,29 @@
continue;
}
if (isReferenced(model, wrapper)) {
kept.add(wrapper.fullName());
kept.add("%s (still referenced)".formatted(wrapper.fullName()));
continue;
}
if (removeClass(model, wrapper)) {
removed.add(wrapper.fullName());
} else {
kept.add(wrapper.fullName());
kept.add("%s (removeClass failed)".formatted(wrapper.fullName()));
}
}

if (!removed.isEmpty()) {
log.info("Removed wrapper classes:\n {}", String.join("\n ", removed));
log.info("Removed {} wrapper class(es)", removed.size());
log.debug(
"Removed wrapper class(es):\n {}",
String.join("\n ", removed)
);
}
if (!kept.isEmpty()) {
log.warn("Skipped removing wrapper classes:\n {}", String.join("\n ", kept));
log.warn(
"Skipped removing {} wrapper class(es):\n {}",
kept.size(),
String.join("\n ", kept)
);
}
}

Expand Down Expand Up @@ -467,8 +497,11 @@
var classOutline = outline.getClazz(flattened.owner());
var field = classOutline.implClass.fields().get(flattened.propertyName());
if (field == null) {
log.warn("Could not find field {} on {}",
flattened.propertyName(), flattened.owner().fullName());
log.warn(
"Could not find field {} on {}",
flattened.propertyName(),
flattened.owner().fullName()
);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
* <strong>Mutual exclusion:</strong> do not enable this plugin together with
* {@link RenameMultiElementPropPlugin} — one renames while the other splits.
* </p>
* <p>
* Logging: each successful split is {@code DEBUG}; the property count is {@code INFO}.
* Failed {@code addProperty} / empty replacement cases are single-line {@code WARN}.
* </p>
*
* @author Rawvoid
*/
Expand Down Expand Up @@ -112,7 +116,7 @@
flattened += handleClass(bean);
}
if (flattened > 0) {
log.info("Flattened {} multi-element property(ies) into individual fields", flattened);
log.info("Flattened {} multi-element property(ies)", flattened);
}
}

Expand All @@ -137,12 +141,20 @@
continue;
}

var originalName = prop.getName(false);
// Replace the original property at position i with all the new ones.
if (replaceProperty(bean, i, prop, replacements)) {
var addedNames = replaceProperty(bean, i, prop, replacements);
if (!addedNames.isEmpty()) {
// After replacement, i now points at the first new property.
// Advance past all inserted properties so the loop continues correctly.
i += replacements.size() - 1;
i += addedNames.size() - 1;

Check warning on line 150 in plugins/src/main/java/io/github/rawvoid/jaxb/plugin/FlattenMultiElementPropPlugin.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor the code in order to not assign to this loop counter from within the loop body.

See more on https://sonarcloud.io/project/issues?id=rawvoid_jaxb-plugins&issues=AZ_B8IecUItUTMcR3nVD&open=AZ_B8IecUItUTMcR3nVD&pullRequest=201
count++;
log.debug(
"Flattened {}.{} → {}",
bean.fullName(),
originalName,
addedNames
);
}
}
return count;
Expand Down Expand Up @@ -360,43 +372,51 @@
* new property. The appended properties are then moved from the tail back to
* the original position.
* </p>
*
* @return short names of properties that were actually appended (empty if none)
*/
private boolean replaceProperty(
private List<String> replaceProperty(
CClassInfo owner, int index, CPropertyInfo original,
List<CPropertyInfo> replacements
) {
var properties = owner.getProperties();
var sizeBefore = properties.size();

// 1. Append all replacements via addProperty (triggers setParent).
var added = 0;
var addedNames = new ArrayList<String>();
for (var replacement : replacements) {
owner.addProperty(replacement);
if (properties.size() == sizeBefore + added + 1) {
added++;
if (properties.size() == sizeBefore + addedNames.size() + 1) {
addedNames.add(replacement.getName(false));
} else {
log.warn("Skip adding {}.{}: addProperty did not append",
owner.fullName(), replacement.getName(false));
log.warn(
"Skip adding {}.{}: addProperty did not append",
owner.fullName(),
replacement.getName(false)
);
}
}

if (added == 0) {
log.warn("Skip flattening {}.{}: no replacements were added",
owner.fullName(), original.getName(false));
return false;
if (addedNames.isEmpty()) {
log.warn(
"Skip flattening {}.{}: no replacements were added",
owner.fullName(),
original.getName(false)
);
return List.of();
}

// 2. Remove original at index.
properties.remove(index);

// 3. Move the `added` properties from the tail to index.
// 3. Move the appended properties from the tail to index.
// Since removeLast() retrieves the last added property first, inserting
// them successively at `index` naturally restores their original order.
for (var j = 0; j < added; j++) {
for (var j = 0; j < addedNames.size(); j++) {
properties.add(index, properties.removeLast());
}

return true;
return addedNames;
}

/**
Expand Down
Loading