From ac3a04252db0c6b6dedb37a6cf7eaf232cd959a9 Mon Sep 17 00:00:00 2001 From: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> Date: Fri, 26 Jun 2026 15:12:09 +0300 Subject: [PATCH 1/3] Warn when @PluginBuilderAttribute field lacks public setter The annotation processor now validates that every @PluginBuilderAttributevfield in a plugin builder class has an accessible public setter method (setXxx or withXxx). If no setter is found, a compilation ERROR is emitted. Use @SuppressWarnings("log4j.public.setter") on the field to suppress. Added @SuppressWarnings to known false positives in: - SocketPerformancePreferences (3 fields: setters return void, not builder) - StringMatchFilter (field 'text' has setter named 'setMatchString') - Rfc5424Layout (field 'enterpriseNumber' has no setter at all) Signed-off-by: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> --- .../log4j/core/filter/StringMatchFilter.java | 1 + .../log4j/core/layout/Rfc5424Layout.java | 1 + .../net/SocketPerformancePreferences.java | 3 + .../plugin/processor/PluginProcessor.java | 93 +++++++++++++- .../PluginProcessorPublicSetterTest.java | 117 ++++++++++++++++++ .../FakePluginPublicSetter.java.source | 52 ++++++++ 6 files changed, 264 insertions(+), 3 deletions(-) create mode 100644 log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java create mode 100644 log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java index 51ee3b2a201..42e03e8de83 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/StringMatchFilter.java @@ -237,6 +237,7 @@ public static StringMatchFilter.Builder newBuilder() { public static class Builder extends AbstractFilterBuilder implements org.apache.logging.log4j.core.util.Builder { @PluginBuilderAttribute + @SuppressWarnings("log4j.public.setter") private String text = ""; /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java index 11bbccb8c54..5d09c11156b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/Rfc5424Layout.java @@ -634,6 +634,7 @@ public static class Rfc5424LayoutBuilder extends AbstractStringLayout.Builder */ @NullMarked -@SupportedAnnotationTypes("org.apache.logging.log4j.plugins.Plugin") +@SupportedAnnotationTypes({ + "org.apache.logging.log4j.plugins.Plugin", + "org.apache.logging.log4j.plugins.PluginBuilderAttribute", + "org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute" +}) @ServiceProvider(value = Processor.class, resolution = Resolution.OPTIONAL) public class PluginProcessor extends AbstractProcessor { @@ -92,6 +102,8 @@ public class PluginProcessor extends AbstractProcessor { */ public static final String PLUGIN_PACKAGE = "log4j.plugin.package"; + private static final String SUPPRESS_WARNING_PUBLIC_SETTER = "log4j.public.setter"; + private static final String SERVICE_FILE_NAME = "META-INF/services/org.apache.logging.log4j.plugins.model.PluginService"; @@ -121,8 +133,16 @@ public synchronized void init(ProcessingEnvironment processingEnv) { @Override public boolean process(final Set annotations, final RoundEnvironment roundEnv) { // Process the elements for this round - if (!annotations.isEmpty()) { - processPluginAnnotatedClasses(ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(Plugin.class))); + for (TypeElement annotation : annotations) { + final String fqn = annotation.getQualifiedName().toString(); + if (fqn.equals("org.apache.logging.log4j.plugins.Plugin")) { + processPluginAnnotatedClasses( + ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(annotation))); + } else if (fqn.equals("org.apache.logging.log4j.plugins.PluginBuilderAttribute") + || fqn.equals("org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute")) { + processBuilderAttributeFields( + roundEnv.getElementsAnnotatedWith(annotation)); + } } // Write the generated code if (roundEnv.processingOver() && !pluginIndex.isEmpty()) { @@ -164,6 +184,73 @@ private void processPluginAnnotatedClasses(Set pluginClasses) { } } + private void processBuilderAttributeFields(final Iterable elements) { + for (final Element element : elements) { + if (element instanceof VariableElement) { + processBuilderAttributeField((VariableElement) element); + } + } + } + + private void processBuilderAttributeField(final VariableElement element) { + final String fieldName = element.getSimpleName().toString(); + // Check for @SuppressWarnings("log4j.public.setter") + final SuppressWarnings suppress = element.getAnnotation(SuppressWarnings.class); + if (suppress != null + && Arrays.asList(suppress.value()).contains(SUPPRESS_WARNING_PUBLIC_SETTER)) { + return; + } + final Element enclosingElement = element.getEnclosingElement(); + // element is a field, its enclosing element is a type + if (enclosingElement instanceof TypeElement) { + final TypeElement typeElement = (TypeElement) enclosingElement; + // Check the siblings of the field for a matching setter + for (final Element enclosedElement : typeElement.getEnclosedElements()) { + if (enclosedElement instanceof ExecutableElement) { + final ExecutableElement methodElement = (ExecutableElement) enclosedElement; + final String methodName = methodElement.getSimpleName().toString(); + if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") + || methodName.toLowerCase(Locale.ROOT).startsWith("with")) + && methodElement.getParameters().size() == 1) { + final Types typeUtils = processingEnv.getTypeUtils(); + final boolean followsNamePattern = methodName.equals( + String.format("set%s", expectedFieldNameInASetter(fieldName))) + || methodName.equals(String.format("with%s", expectedFieldNameInASetter(fieldName))); + final boolean isPublicMethod = + methodElement.getModifiers().contains(Modifier.PUBLIC); + final boolean checkForAssignable = typeUtils.isAssignable( + methodElement.getReturnType(), + methodElement.getEnclosingElement().asType()); + final boolean foundPublicSetter = + followsNamePattern && checkForAssignable && isPublicMethod; + if (foundPublicSetter) { + return; + } + } + } + } + // No setter found: emit a compilation error + processingEnv + .getMessager() + .printMessage( + javax.tools.Diagnostic.Kind.ERROR, + String.format( + "The field `%s` does not have a public setter. " + + "Note that @SuppressWarnings(\"%s\") can be used on the field to suppress this error.", + fieldName, SUPPRESS_WARNING_PUBLIC_SETTER), + element); + } + } + + private static String expectedFieldNameInASetter(String fieldName) { + if (fieldName.startsWith("is")) { + fieldName = fieldName.substring(2); + } + return fieldName.isEmpty() + ? fieldName + : Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1); + } + private static void processConfigurableAnnotation(TypeElement pluginClass, PluginEntry.Builder builder) { var configurable = pluginClass.getAnnotation(Configurable.class); if (configurable != null) { diff --git a/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java new file mode 100644 index 00000000000..4e9f6994291 --- /dev/null +++ b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java @@ -0,0 +1,117 @@ +/* + * 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.logging.log4j.plugin.processor; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import java.util.stream.Collectors; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticCollector; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; +import org.apache.commons.io.FileUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class PluginProcessorPublicSetterTest { + + private static final String FAKE_PLUGIN_CLASS_PATH = + "src/test/resources/setter-test/FakePluginPublicSetter.java.source"; + + private File createdFile; + private DiagnosticCollector diagnosticCollector; + private List> errorDiagnostics; + + @BeforeEach + void setup() { + final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + diagnosticCollector = new DiagnosticCollector<>(); + final StandardJavaFileManager fileManager = + compiler.getStandardFileManager(null, Locale.ROOT, UTF_8); + + final Path sourceFile = Paths.get(FAKE_PLUGIN_CLASS_PATH); + + assertThat(sourceFile).exists(); + + final File orig = sourceFile.toFile(); + createdFile = new File(orig.getParentFile(), "FakePluginPublicSetter.java"); + assertDoesNotThrow(() -> FileUtils.copyFile(orig, createdFile)); + + final Iterable compilationUnits = + fileManager.getJavaFileObjects(createdFile); + + final JavaCompiler.CompilationTask task = compiler.getTask( + null, + fileManager, + diagnosticCollector, + Arrays.asList("-proc:only", "-processor", PluginProcessor.class.getName()), + null, + compilationUnits); + task.call(); + + errorDiagnostics = diagnosticCollector.getDiagnostics().stream() + .filter(diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR) + .collect(Collectors.toList()); + } + + @AfterEach + void tearDown() { + assertDoesNotThrow(() -> FileUtils.delete(createdFile)); + // Clean up generated processor artifacts + final File pluginsDatFile = Paths.get("Log4j2Plugins.dat").toFile(); + if (pluginsDatFile.exists()) { + assertDoesNotThrow(() -> FileUtils.delete(pluginsDatFile)); + } + } + + @Test + void warnWhenPluginBuilderAttributeLacksPublicSetter() { + assertThat(errorDiagnostics) + .anyMatch(errorMessage -> errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attributeWithoutPublicSetter` does not have a public setter")); + } + + @Test + void ignoreWarningWhenSuppressWarningsIsPresent() { + assertThat(errorDiagnostics) + .allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains( + "The field `attributeWithoutPublicSetterButWithSuppressAnnotation`" + + " does not have a public setter")); + } + + @Test + void noWarningWhenPublicSetterExists() { + assertThat(errorDiagnostics) + .allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attribute` does not have a public setter")); + } +} diff --git a/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source b/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source new file mode 100644 index 00000000000..6a7b9c12391 --- /dev/null +++ b/log4j-plugin-processor/src/test/resources/setter-test/FakePluginPublicSetter.java.source @@ -0,0 +1,52 @@ +/* + * 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 example; + +import org.apache.logging.log4j.plugins.Plugin; +import org.apache.logging.log4j.plugins.PluginBuilderAttribute; + +/** + * Test plugin class for unit tests of public setter validation. + */ +@Plugin("FakePluginPublicSetter") +public class FakePluginPublicSetter { + + public static class Builder { + + @PluginBuilderAttribute + private int attribute; + + @PluginBuilderAttribute + @SuppressWarnings("log4j.public.setter") + private int attributeWithoutPublicSetterButWithSuppressAnnotation; + + @PluginBuilderAttribute + private int attributeWithoutPublicSetter; + + public Builder setAttribute(final int attribute) { + this.attribute = attribute; + return this; + } + + public Builder setAttributeWithoutPublicSetterButWithSuppressAnnotation( + final int attributeWithoutPublicSetterButWithSuppressAnnotation) { + this.attributeWithoutPublicSetterButWithSuppressAnnotation = + attributeWithoutPublicSetterButWithSuppressAnnotation; + return this; + } + } +} From a68450c1fdf0ca62d4422729316816400da28f0b Mon Sep 17 00:00:00 2001 From: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:57:52 +0300 Subject: [PATCH 2/3] Fix JdbcAppender builder setters to return fluent builder type The annotation processor requires @PluginBuilderAttribute setters to return a type assignable to the enclosing builder class. Changed setImmediateFail() and setReconnectIntervalMillis() from void to B, following the same pattern as all other setters in the Builder class. Fixes compilation errors caught by the new annotation processor validation: - The field immediateFail does not have a public setter. - The field reconnectIntervalMillis does not have a public setter. Signed-off-by: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> --- .../apache/logging/log4j/jdbc/appender/JdbcAppender.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/log4j-jdbc/src/main/java/org/apache/logging/log4j/jdbc/appender/JdbcAppender.java b/log4j-jdbc/src/main/java/org/apache/logging/log4j/jdbc/appender/JdbcAppender.java index 80122ffd615..e169d240a9e 100644 --- a/log4j-jdbc/src/main/java/org/apache/logging/log4j/jdbc/appender/JdbcAppender.java +++ b/log4j-jdbc/src/main/java/org/apache/logging/log4j/jdbc/appender/JdbcAppender.java @@ -158,12 +158,14 @@ public B setConnectionSource(final ConnectionSource connectionSource) { return asBuilder(); } - public void setImmediateFail(final boolean immediateFail) { + public B setImmediateFail(final boolean immediateFail) { this.immediateFail = immediateFail; + return asBuilder(); } - public void setReconnectIntervalMillis(final long reconnectIntervalMillis) { + public B setReconnectIntervalMillis(final long reconnectIntervalMillis) { this.reconnectIntervalMillis = reconnectIntervalMillis; + return asBuilder(); } /** From a38185a135a2d9b9d84a53a3f6e16b8441648bd4 Mon Sep 17 00:00:00 2001 From: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:07:33 +0300 Subject: [PATCH 3/3] Apply spotless formatting to PluginProcessor and its test Fixes formatting violations caught by spotless:check: - PluginProcessor: line wrapping, duplicate import removal - PluginProcessorPublicSetterTest: line wrapping consistency Signed-off-by: Vasily Pelikh <2010720+vpelikh@users.noreply.github.com> --- .../plugin/processor/PluginProcessor.java | 19 ++++-------- .../PluginProcessorPublicSetterTest.java | 30 ++++++++----------- 2 files changed, 18 insertions(+), 31 deletions(-) diff --git a/log4j-plugin-processor/src/main/java/org/apache/logging/log4j/plugin/processor/PluginProcessor.java b/log4j-plugin-processor/src/main/java/org/apache/logging/log4j/plugin/processor/PluginProcessor.java index 12def7f2e3e..d4dfbbf0790 100644 --- a/log4j-plugin-processor/src/main/java/org/apache/logging/log4j/plugin/processor/PluginProcessor.java +++ b/log4j-plugin-processor/src/main/java/org/apache/logging/log4j/plugin/processor/PluginProcessor.java @@ -40,7 +40,6 @@ import javax.annotation.processing.SupportedAnnotationTypes; import javax.lang.model.SourceVersion; import javax.lang.model.element.AnnotationValue; -import javax.lang.model.element.AnnotationValue; import javax.lang.model.element.Element; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.Modifier; @@ -136,12 +135,10 @@ public boolean process(final Set annotations, final Round for (TypeElement annotation : annotations) { final String fqn = annotation.getQualifiedName().toString(); if (fqn.equals("org.apache.logging.log4j.plugins.Plugin")) { - processPluginAnnotatedClasses( - ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(annotation))); + processPluginAnnotatedClasses(ElementFilter.typesIn(roundEnv.getElementsAnnotatedWith(annotation))); } else if (fqn.equals("org.apache.logging.log4j.plugins.PluginBuilderAttribute") || fqn.equals("org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute")) { - processBuilderAttributeFields( - roundEnv.getElementsAnnotatedWith(annotation)); + processBuilderAttributeFields(roundEnv.getElementsAnnotatedWith(annotation)); } } // Write the generated code @@ -196,8 +193,7 @@ private void processBuilderAttributeField(final VariableElement element) { final String fieldName = element.getSimpleName().toString(); // Check for @SuppressWarnings("log4j.public.setter") final SuppressWarnings suppress = element.getAnnotation(SuppressWarnings.class); - if (suppress != null - && Arrays.asList(suppress.value()).contains(SUPPRESS_WARNING_PUBLIC_SETTER)) { + if (suppress != null && Arrays.asList(suppress.value()).contains(SUPPRESS_WARNING_PUBLIC_SETTER)) { return; } final Element enclosingElement = element.getEnclosingElement(); @@ -214,15 +210,14 @@ private void processBuilderAttributeField(final VariableElement element) { && methodElement.getParameters().size() == 1) { final Types typeUtils = processingEnv.getTypeUtils(); final boolean followsNamePattern = methodName.equals( - String.format("set%s", expectedFieldNameInASetter(fieldName))) + String.format("set%s", expectedFieldNameInASetter(fieldName))) || methodName.equals(String.format("with%s", expectedFieldNameInASetter(fieldName))); final boolean isPublicMethod = methodElement.getModifiers().contains(Modifier.PUBLIC); final boolean checkForAssignable = typeUtils.isAssignable( methodElement.getReturnType(), methodElement.getEnclosingElement().asType()); - final boolean foundPublicSetter = - followsNamePattern && checkForAssignable && isPublicMethod; + final boolean foundPublicSetter = followsNamePattern && checkForAssignable && isPublicMethod; if (foundPublicSetter) { return; } @@ -246,9 +241,7 @@ private static String expectedFieldNameInASetter(String fieldName) { if (fieldName.startsWith("is")) { fieldName = fieldName.substring(2); } - return fieldName.isEmpty() - ? fieldName - : Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1); + return fieldName.isEmpty() ? fieldName : Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1); } private static void processConfigurableAnnotation(TypeElement pluginClass, PluginEntry.Builder builder) { diff --git a/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java index 4e9f6994291..15d41674c67 100644 --- a/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java +++ b/log4j-plugin-processor/src/test/java/org/apache/logging/log4j/plugin/processor/PluginProcessorPublicSetterTest.java @@ -51,8 +51,7 @@ public class PluginProcessorPublicSetterTest { void setup() { final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); diagnosticCollector = new DiagnosticCollector<>(); - final StandardJavaFileManager fileManager = - compiler.getStandardFileManager(null, Locale.ROOT, UTF_8); + final StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, Locale.ROOT, UTF_8); final Path sourceFile = Paths.get(FAKE_PLUGIN_CLASS_PATH); @@ -62,8 +61,7 @@ void setup() { createdFile = new File(orig.getParentFile(), "FakePluginPublicSetter.java"); assertDoesNotThrow(() -> FileUtils.copyFile(orig, createdFile)); - final Iterable compilationUnits = - fileManager.getJavaFileObjects(createdFile); + final Iterable compilationUnits = fileManager.getJavaFileObjects(createdFile); final JavaCompiler.CompilationTask task = compiler.getTask( null, @@ -91,27 +89,23 @@ void tearDown() { @Test void warnWhenPluginBuilderAttributeLacksPublicSetter() { - assertThat(errorDiagnostics) - .anyMatch(errorMessage -> errorMessage - .getMessage(Locale.ROOT) - .contains("The field `attributeWithoutPublicSetter` does not have a public setter")); + assertThat(errorDiagnostics).anyMatch(errorMessage -> errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attributeWithoutPublicSetter` does not have a public setter")); } @Test void ignoreWarningWhenSuppressWarningsIsPresent() { - assertThat(errorDiagnostics) - .allMatch(errorMessage -> !errorMessage - .getMessage(Locale.ROOT) - .contains( - "The field `attributeWithoutPublicSetterButWithSuppressAnnotation`" - + " does not have a public setter")); + assertThat(errorDiagnostics).allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attributeWithoutPublicSetterButWithSuppressAnnotation`" + + " does not have a public setter")); } @Test void noWarningWhenPublicSetterExists() { - assertThat(errorDiagnostics) - .allMatch(errorMessage -> !errorMessage - .getMessage(Locale.ROOT) - .contains("The field `attribute` does not have a public setter")); + assertThat(errorDiagnostics).allMatch(errorMessage -> !errorMessage + .getMessage(Locale.ROOT) + .contains("The field `attribute` does not have a public setter")); } }