gnodet-bot commented on code in PR #12695: URL: https://github.com/apache/maven/pull/12695#discussion_r4051039302
########## api/maven-api-core/src/main/java/org/apache/maven/api/BuildEnvironment.java: ########## @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.api; + +import java.util.List; +import java.util.Map; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Immutable; +import org.apache.maven.api.annotations.Nonnull; + +/** + * Describes the invocation context of a Maven build: the flags, properties, and + * environment settings that were active when the build started. + * + * <p>An instance is available via {@link Session#buildEnvironment()} during the build, + * and is also recorded in the structured build report for post-mortem analysis and + * reproducibility. + * + * <h2>What is captured</h2> + * <ul> + * <li>Goals and lifecycle phases requested ({@link #goals()})</li> + * <li>User properties passed via {@code -Dkey=value} ({@link #userProperties()}), + * with sensitive keys redacted — see {@link #userProperties()} for the denylist</li> + * <li>A curated subset of system properties relevant to reproducibility + * ({@link #systemInfo()}): OS name/arch/version, Java vendor and VM name/version, + * Maven home, and available processors</li> + * <li>Local repository path ({@link #localRepository()})</li> + * <li>Explicitly activated or deactivated profiles ({@link #activeProfiles()})</li> + * <li>Selected projects ({@link #selectedProjects()}) and resume-from + * ({@link #resumeFrom()})</li> + * <li>Reactor failure behavior ({@link #reactorFailureBehavior()})</li> + * <li>Offline mode ({@link #offline()}) and snapshot update policy + * ({@link #updateSnapshots()})</li> + * <li>Degree of concurrency ({@link #threads()})</li> + * </ul> + * + * <h2>What is not yet captured</h2> + * <p>The following information is not currently available through the Maven 4 API + * and therefore cannot be recorded here. It may be added in future versions as + * the API evolves: + * <ul> + * <li><b>Raw command-line arguments</b> ({@code args[]}): the CLI bootstrap layer + * does not surface these through {@code MavenExecutionRequest} or the session</li> + * <li><b>Batch mode</b> ({@code -B} / {@code --batch-mode}): not yet exposed on + * the Maven 4 {@code Session} API; currently only available via + * {@code MavenExecutionRequest.isInteractiveMode()} in the compat layer</li> + * <li><b>No-transfer-progress</b> ({@code --no-transfer-progress}): not yet + * exposed on the Maven 4 {@code Session} API</li> + * <li><b>Implicitly activated profiles</b>: profiles activated by OS, JDK, or + * property conditions rather than by explicit {@code -P} flag; computing Review Comment: **[Medium] Stale "What is not yet captured" section.** Both `batchMode` and `noTransferProgress` are now exposed as methods on `BuildEnvironment` (lines 187-199 of this file), but they still appear in the "What is not yet captured" list as if they are not available. Remove the `<li>` entries for Batch mode and No-transfer-progress from the `<h2>What is not yet captured</h2>` section. _This review was generated by an AI agent, Hermès on behalf of @gnodet._ <!-- reviewer: gnodet-bot --> ########## api/maven-api-core/src/main/java/org/apache/maven/api/BuildEnvironment.java: ########## @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.api; + +import java.util.List; +import java.util.Map; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Immutable; +import org.apache.maven.api.annotations.Nonnull; + +/** + * Describes the invocation context of a Maven build: the flags, properties, and + * environment settings that were active when the build started. + * + * <p>An instance is available via {@link Session#buildEnvironment()} during the build, + * and is also recorded in the structured build report for post-mortem analysis and + * reproducibility. + * + * <h2>What is captured</h2> + * <ul> + * <li>Goals and lifecycle phases requested ({@link #goals()})</li> + * <li>User properties passed via {@code -Dkey=value} ({@link #userProperties()}), + * with sensitive keys redacted — see {@link #userProperties()} for the denylist</li> + * <li>A curated subset of system properties relevant to reproducibility + * ({@link #systemInfo()}): OS name/arch/version, Java vendor and VM name/version, + * Maven home, and available processors</li> + * <li>Local repository path ({@link #localRepository()})</li> + * <li>Explicitly activated or deactivated profiles ({@link #activeProfiles()})</li> + * <li>Selected projects ({@link #selectedProjects()}) and resume-from + * ({@link #resumeFrom()})</li> + * <li>Reactor failure behavior ({@link #reactorFailureBehavior()})</li> + * <li>Offline mode ({@link #offline()}) and snapshot update policy + * ({@link #updateSnapshots()})</li> + * <li>Degree of concurrency ({@link #threads()})</li> + * </ul> + * + * <h2>What is not yet captured</h2> + * <p>The following information is not currently available through the Maven 4 API + * and therefore cannot be recorded here. It may be added in future versions as + * the API evolves: + * <ul> + * <li><b>Raw command-line arguments</b> ({@code args[]}): the CLI bootstrap layer + * does not surface these through {@code MavenExecutionRequest} or the session</li> + * <li><b>Batch mode</b> ({@code -B} / {@code --batch-mode}): not yet exposed on + * the Maven 4 {@code Session} API; currently only available via + * {@code MavenExecutionRequest.isInteractiveMode()} in the compat layer</li> + * <li><b>No-transfer-progress</b> ({@code --no-transfer-progress}): not yet + * exposed on the Maven 4 {@code Session} API</li> Review Comment: **[Medium, RERAISED x4] Stale Javadoc — `batchMode` and `noTransferProgress` still listed as "not yet captured".** Lines 61–65 of the class-level Javadoc say: > *Batch mode (`-B` / `--batch-mode`): not yet exposed on the Maven 4 `Session` API* > *No-transfer-progress (`--no-transfer-progress`): not yet exposed on the Maven 4 `Session` API* But lines 187–199 in this same file now declare `boolean noTransferProgress()` and `boolean batchMode()` as interface methods — they are exposed. The Javadoc is actively wrong. Remove these two bullet points from the "What is not yet captured" section. ########## impl/maven-core/src/main/java/org/apache/maven/internal/build/BuildReportCollector.java: ########## @@ -0,0 +1,664 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.internal.build; + +import javax.inject.Named; +import javax.inject.Singleton; + +import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.nio.file.AtomicMoveNotSupportedException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.time.Duration; +import java.time.Instant; +import java.time.ZoneOffset; +import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.api.MonotonicClock; +import org.apache.maven.api.build.report.BuildReport; +import org.apache.maven.api.build.report.BuildStatus; +import org.apache.maven.api.build.report.FailureReport; +import org.apache.maven.api.build.report.LogEvent; +import org.apache.maven.api.build.report.ModuleReport; +import org.apache.maven.api.build.report.MojoReport; +import org.apache.maven.eventspy.AbstractEventSpy; +import org.apache.maven.execution.BuildFailure; +import org.apache.maven.execution.BuildSuccess; +import org.apache.maven.execution.BuildSummary; +import org.apache.maven.execution.ExecutionEvent; +import org.apache.maven.execution.MavenExecutionResult; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.logging.ProjectBuildLogAppender; +import org.apache.maven.plugin.MojoExecution; +import org.apache.maven.project.MavenProject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Collects build lifecycle events and produces a structured {@link BuildReport} + * at the end of the session. + * <p> + * Registered as an {@link org.apache.maven.eventspy.EventSpy} via {@code @Named}/{@code @Singleton}, + * following the same pattern as {@code DefaultPluginValidationManager}. + * <p> + * Thread-safe: concurrent module builds (with {@code -T}) each write to their + * own entry in a {@link ConcurrentHashMap}. + * <p> + * Log capture: registers a callback on {@link ProjectBuildLogAppender} to + * receive the already-formed {@link LogEvent} objects produced by the main + * logging pipeline. Uses thread-based tracking to associate events with + * the currently-executing mojo or module. + * + * @since 4.1.0 + */ +@Singleton +@Named +public final class BuildReportCollector extends AbstractEventSpy { + + private static final Logger LOGGER = LoggerFactory.getLogger(BuildReportCollector.class); + + static final String REPORT_DIR = "build-reports"; + static final String REPORT_LATEST = "build-report-latest.json"; + + private static final int MAX_STACKTRACE_LINES = 30; + + /** + * Maximum number of log events captured per scope (mojo, module, or build). + * Beyond this, events are dropped and a truncation notice is appended. Review Comment: **[Medium, RERAISED x5] Javadoc says "truncation notice is appended" but none is ever appended.** Line 95: *"Beyond this, events are dropped and a truncation notice is appended."* `captureLogEvent()` (lines 289, 299, 306) simply skips events beyond the limit — there is no `else` branch and no sentinel event appended. A build report consumer that reads a 500-event list has no way to know whether the output was complete or silently truncated. Fix: either implement the truncation notice (append a synthetic `LogEvent` with `WARN` level and a message like `"... N events truncated"`), or change the Javadoc to say "events are silently dropped". ########## api/maven-api-core/src/main/java/org/apache/maven/api/Session.java: ########## @@ -46,6 +46,20 @@ @ThreadSafe public interface Session extends ProtoSession { + /** + * Returns the environment context of this build: the flags, properties, and + * platform settings that were active when the session started. + * + * <p>The returned instance is immutable and captures a snapshot of the invocation + * context (offline mode, user properties, selected projects, etc.). It is the same + * object recorded in the structured build report. Review Comment: **[Medium, RERAISED x3] Javadoc says "It is the same object recorded in the structured build report" — but it is not.** `DefaultSession.buildEnvironment()` calls `BuildReportCollector.buildEnvironment(getMavenSession())` — a static method that constructs a **fresh** `DefaultBuildEnvironment` on every call, reading system properties and iterating user properties to redact sensitive keys. There is no caching. The object returned here is never the same instance stored in the `BuildReport`. Fix: either cache the instance in `DefaultSession` (populated at `SessionStarted`), or update the Javadoc to say "a snapshot of the build environment" instead of "the same object". ########## impl/maven-core/src/main/java/org/apache/maven/internal/build/BuildReportJsonWriter.java: ########## @@ -0,0 +1,439 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.internal.build; + +import org.apache.maven.api.BuildEnvironment; +import org.apache.maven.api.build.report.BuildReport; +import org.apache.maven.api.build.report.FailureReport; +import org.apache.maven.api.build.report.LogEvent; +import org.apache.maven.api.build.report.ModuleReport; +import org.apache.maven.api.build.report.MojoReport; +import org.apache.maven.api.services.BuilderProblem; + +/** + * Serializes a {@link BuildReport} to JSON without any external library dependency. + * <p> + * The output is human-readable (indented with 2 spaces) and designed to be + * stable across Maven versions — field order is fixed, and new fields are + * appended at the end of each object. + */ +final class BuildReportJsonWriter { + + private BuildReportJsonWriter() {} + + /** + * Serialize the given report to a pretty-printed JSON string. + */ + static String toJson(BuildReport report) { + StringBuilder sb = new StringBuilder(4096); + writeReport(sb, report, 0); + sb.append('\n'); + return sb.toString(); + } + + private static void writeReport(StringBuilder sb, BuildReport report, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "formatVersion", report.formatVersion()); + writeField(sb, indent + 1, "status", report.status().name()); + writeField(sb, indent + 1, "duration", report.duration().toString()); + writeField(sb, indent + 1, "startTime", report.startTime().toString()); + writeField(sb, indent + 1, "mavenVersion", report.mavenVersion()); + writeField(sb, indent + 1, "javaVersion", report.javaVersion()); + writeStringArray(sb, indent + 1, "goals", report.goals()); + writeField(sb, indent + 1, "project", report.project()); + writeField(sb, indent + 1, "multiModule", report.multiModule()); + writeField(sb, indent + 1, "threads", report.threads()); + + // environment object + writeIndent(sb, indent + 1); + sb.append("\"environment\": "); + writeEnvironment(sb, report.environment(), indent + 1); + sb.append(",\n"); + + // modules array + writeIndent(sb, indent + 1); + sb.append("\"modules\": "); + if (report.modules().isEmpty()) { + sb.append("[]"); + } else { + sb.append("[\n"); + for (int i = 0; i < report.modules().size(); i++) { + writeIndent(sb, indent + 2); + writeModule(sb, report.modules().get(i), indent + 2); + if (i < report.modules().size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent + 1); + sb.append(']'); + } + sb.append(",\n"); + + // failures array + writeIndent(sb, indent + 1); + sb.append("\"failures\": "); + if (report.failures().isEmpty()) { + sb.append("[]"); + } else { + sb.append("[\n"); + for (int i = 0; i < report.failures().size(); i++) { + writeIndent(sb, indent + 2); + writeFailure(sb, report.failures().get(i), indent + 2); + if (i < report.failures().size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent + 1); + sb.append(']'); + } + sb.append(",\n"); + + // problems array + writeIndent(sb, indent + 1); + sb.append("\"problems\": "); + if (report.problems().isEmpty()) { + sb.append("[]"); + } else { + sb.append("[\n"); + for (int i = 0; i < report.problems().size(); i++) { + writeIndent(sb, indent + 2); + writeProblem(sb, report.problems().get(i), indent + 2); + if (i < report.problems().size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent + 1); + sb.append(']'); + } + sb.append(",\n"); + + // output array — build-level log lines (outside any module) + writeOutputArray(sb, indent + 1, report.output()); + sb.append('\n'); + + writeIndent(sb, indent); + sb.append('}'); + } + + private static void writeEnvironment(StringBuilder sb, BuildEnvironment env, int indent) { + sb.append("{\n"); + writeStringArray(sb, indent + 1, "goals", env.goals()); + writeStringMap(sb, indent + 1, "userProperties", env.userProperties()); + writeStringMap(sb, indent + 1, "systemInfo", env.systemInfo()); + writeField(sb, indent + 1, "localRepository", env.localRepository()); + writeStringArray(sb, indent + 1, "activeProfiles", env.activeProfiles()); + writeStringArray(sb, indent + 1, "selectedProjects", env.selectedProjects()); + writeNullableField(sb, indent + 1, "resumeFrom", env.resumeFrom(), true); + writeField(sb, indent + 1, "reactorFailureBehavior", env.reactorFailureBehavior()); + writeField(sb, indent + 1, "offline", env.offline()); + writeField(sb, indent + 1, "updateSnapshots", env.updateSnapshots()); + writeField(sb, indent + 1, "noTransferProgress", env.noTransferProgress()); + writeField(sb, indent + 1, "batchMode", env.batchMode()); + writeField(sb, indent + 1, "threads", env.threads()); + removeTrailingComma(sb); + writeIndent(sb, indent); + sb.append('}'); + } + + private static void writeStringMap(StringBuilder sb, int indent, String key, java.util.Map<String, String> map) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": "); + if (map.isEmpty()) { + sb.append("{}"); + } else { + sb.append("{\n"); + var entries = new java.util.ArrayList<>(map.entrySet()); + for (int i = 0; i < entries.size(); i++) { + var entry = entries.get(i); + writeIndent(sb, indent + 1); + writeJsonString(sb, entry.getKey()); + sb.append(": "); + writeJsonString(sb, entry.getValue()); + if (i < entries.size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent); + sb.append('}'); + } + sb.append(",\n"); + } + + private static void writeProblem(StringBuilder sb, BuilderProblem problem, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "severity", problem.getSeverity().name()); + writeField(sb, indent + 1, "message", problem.getMessage()); + String source = problem.getSource(); + if (source != null && !source.isEmpty()) { + writeField(sb, indent + 1, "source", source); + } + if (problem.getLineNumber() > 0) { + writeField(sb, indent + 1, "line", problem.getLineNumber()); + } + if (problem.getColumnNumber() > 0) { + writeField(sb, indent + 1, "column", problem.getColumnNumber()); + } + // Remove the trailing comma from the last written field + int lastComma = sb.lastIndexOf(",\n"); + if (lastComma > 0) { + sb.replace(lastComma, lastComma + 1, ""); + } + writeIndent(sb, indent); + sb.append('}'); + } + + private static void writeModule(StringBuilder sb, ModuleReport module, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "groupId", module.groupId()); + writeField(sb, indent + 1, "artifactId", module.artifactId()); + writeField(sb, indent + 1, "version", module.version()); + writeField(sb, indent + 1, "status", module.status().name()); + writeField(sb, indent + 1, "startTime", module.startTime().toString()); + writeField(sb, indent + 1, "duration", module.duration().toString()); + + // mojos array + writeIndent(sb, indent + 1); + sb.append("\"mojos\": "); + if (module.mojos().isEmpty()) { + sb.append("[]"); + } else { + sb.append("[\n"); + for (int i = 0; i < module.mojos().size(); i++) { + writeIndent(sb, indent + 2); + writeMojo(sb, module.mojos().get(i), indent + 2); + if (i < module.mojos().size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent + 1); + sb.append(']'); + } + sb.append(",\n"); + + // output array — module-level log lines (between mojos) + writeOutputArray(sb, indent + 1, module.output()); + sb.append('\n'); + + writeIndent(sb, indent); + sb.append('}'); + } + + private static void writeMojo(StringBuilder sb, MojoReport mojo, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "groupId", mojo.groupId()); + writeField(sb, indent + 1, "artifactId", mojo.artifactId()); + writeField(sb, indent + 1, "version", mojo.version()); + writeField(sb, indent + 1, "goal", mojo.goal()); + writeNullableField(sb, indent + 1, "executionId", mojo.executionId(), true); + writeNullableField(sb, indent + 1, "phase", mojo.phase(), true); + writeField(sb, indent + 1, "status", mojo.status().name()); + writeField(sb, indent + 1, "startTime", mojo.startTime().toString()); + writeField(sb, indent + 1, "duration", mojo.duration().toString()); + + // output array — captured log lines + writeOutputArray(sb, indent + 1, mojo.output()); + sb.append('\n'); + + writeIndent(sb, indent); + sb.append('}'); + } + + private static void writeFailure(StringBuilder sb, FailureReport failure, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "module", failure.module()); + writeNullableField(sb, indent + 1, "mojo", failure.mojo(), true); + writeField(sb, indent + 1, "timestamp", failure.timestamp().toString()); + writeNullableField(sb, indent + 1, "exceptionType", failure.exceptionType(), true); + if (failure.stackTrace() != null) { + writeField(sb, indent + 1, "message", failure.message()); + writeLastField(sb, indent + 1, "stackTrace", failure.stackTrace()); + } else { + writeLastField(sb, indent + 1, "message", failure.message()); + } + writeIndent(sb, indent); + sb.append('}'); + } + + /** + * Writes an {@code "output": [...]} array of structured log events + * (used by report, module, and mojo). + * This is always the last field in its object, so no trailing comma. + */ + private static void writeOutputArray(StringBuilder sb, int indent, java.util.List<LogEvent> events) { + writeIndent(sb, indent); + sb.append("\"output\": "); + if (events.isEmpty()) { + sb.append("[]"); + } else { + sb.append("[\n"); + for (int i = 0; i < events.size(); i++) { + writeIndent(sb, indent + 1); + writeLogEvent(sb, events.get(i), indent + 1); + if (i < events.size() - 1) { + sb.append(','); + } + sb.append('\n'); + } + writeIndent(sb, indent); + sb.append(']'); + } + } + + private static void writeLogEvent(StringBuilder sb, LogEvent event, int indent) { + sb.append("{\n"); + writeField(sb, indent + 1, "timestamp", event.timestamp().toString()); + writeField(sb, indent + 1, "level", event.level().name()); + if (event.loggerName() != null) { + writeField(sb, indent + 1, "loggerName", event.loggerName()); + } + writeField(sb, indent + 1, "message", event.message()); + if (event.stackTrace() != null) { + writeField(sb, indent + 1, "stackTrace", event.stackTrace()); + } + // Source metadata — present for Log API and JUL events + if (event.sourceClassName() != null) { + writeField(sb, indent + 1, "sourceClassName", event.sourceClassName()); + } + if (event.sourceMethodName() != null) { + writeField(sb, indent + 1, "sourceMethodName", event.sourceMethodName()); + } + if (event.threadId() >= 0) { + writeField(sb, indent + 1, "threadId", event.threadId()); + } + if (event.sequenceNumber() >= 0) { + writeField(sb, indent + 1, "sequenceNumber", event.sequenceNumber()); + } + removeTrailingComma(sb); + writeIndent(sb, indent); + sb.append('}'); + } + + // ---- Low-level JSON writing helpers ---- + + private static void writeField(StringBuilder sb, int indent, String key, String value) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": "); + writeJsonString(sb, value); + sb.append(",\n"); + } + + private static void writeField(StringBuilder sb, int indent, String key, int value) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": ").append(value).append(",\n"); + } + + private static void writeField(StringBuilder sb, int indent, String key, long value) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": ").append(value).append(",\n"); + } + + private static void writeField(StringBuilder sb, int indent, String key, boolean value) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": ").append(value).append(",\n"); + } + + /** + * Removes the trailing comma from the last field in a JSON object. + * Turns {@code "field": value,\n} into {@code "field": value\n}. + */ + private static void removeTrailingComma(StringBuilder sb) { + int len = sb.length(); + if (len >= 2 && sb.charAt(len - 2) == ',' && sb.charAt(len - 1) == '\n') { + sb.deleteCharAt(len - 2); + } + } + + private static void writeLastField(StringBuilder sb, int indent, String key, String value) { + writeIndent(sb, indent); + sb.append('"').append(key).append("\": "); + writeJsonString(sb, value); + sb.append('\n'); + } + + private static void writeNullableField( + StringBuilder sb, int indent, String key, String value, @SuppressWarnings("unused") boolean hasMore) { Review Comment: **[Low, RERAISED x5] Dead `hasMore` parameter with `@SuppressWarnings("unused")`.** The `boolean hasMore` parameter is never read — the method always appends `",\n"` unconditionally. Every call site passes `true` hardcoded. The `@SuppressWarnings("unused")` on a parameter you control means the API is wrong, not that the suppression is justified. Remove the parameter and update all call sites. ```suggestion StringBuilder sb, int indent, String key, String value) { ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
