Skip to content

Commit a4dc84d

Browse files
jaykataria1111Jay Katariya
jaykataria1111
authored and
Jay Katariya
committed
Fix(#2769): Modify the annotation processor in 2.x to give a warning if a plugin builder attribute does not have a public setter.
1 parent be2d145 commit a4dc84d

File tree

10 files changed

+317
-0
lines changed

10 files changed

+317
-0
lines changed

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/FakePlugin.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,13 @@ public static Builder newBuilder() {
6060
public static class Builder implements org.apache.logging.log4j.core.util.Builder<FakePlugin> {
6161

6262
@PluginBuilderAttribute
63+
@SuppressWarnings("log4j.public.setter")
6364
private int attribute;
6465

66+
@PluginBuilderAttribute
67+
@SuppressWarnings("log4j.public.setter")
68+
private int attributeWithoutPublicSetterButWithSuppressAnnotation;
69+
6570
@PluginElement("layout")
6671
private Layout<? extends Serializable> layout;
6772

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/FakePluginPublicSetter.java.source

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.config.plugins.processor;
18+
19+
import java.io.Serializable;
20+
import org.apache.logging.log4j.core.Layout;
21+
import org.apache.logging.log4j.core.LoggerContext;
22+
import org.apache.logging.log4j.core.config.Configuration;
23+
import org.apache.logging.log4j.core.config.Node;
24+
import org.apache.logging.log4j.core.config.plugins.Plugin;
25+
import org.apache.logging.log4j.core.config.plugins.PluginAliases;
26+
import org.apache.logging.log4j.core.config.plugins.PluginAttribute;
27+
import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
28+
import org.apache.logging.log4j.core.config.plugins.PluginConfiguration;
29+
import org.apache.logging.log4j.core.config.plugins.PluginElement;
30+
import org.apache.logging.log4j.core.config.plugins.PluginFactory;
31+
import org.apache.logging.log4j.core.config.plugins.PluginLoggerContext;
32+
import org.apache.logging.log4j.core.config.plugins.PluginNode;
33+
import org.apache.logging.log4j.core.config.plugins.PluginValue;
34+
35+
/**
36+
* Test plugin class for unit tests.
37+
*/
38+
@Plugin(name = "Fake", category = "Test")
39+
@PluginAliases({"AnotherFake", "StillFake"})
40+
public class FakePluginPublicSetter {
41+
42+
@Plugin(name = "Nested", category = "Test")
43+
public static class Nested {}
44+
45+
@PluginFactory
46+
public static FakePluginPublicSetter newPlugin(
47+
@PluginAttribute("attribute") int attribute,
48+
@PluginElement("layout") Layout<? extends Serializable> layout,
49+
@PluginConfiguration Configuration config,
50+
@PluginNode Node node,
51+
@PluginLoggerContext LoggerContext loggerContext,
52+
@PluginValue("value") String value) {
53+
return null;
54+
}
55+
56+
public static Builder newBuilder() {
57+
return new Builder();
58+
}
59+
60+
public static class Builder implements org.apache.logging.log4j.core.util.Builder<FakePluginPublicSetter> {
61+
62+
@PluginBuilderAttribute
63+
private int attribute;
64+
65+
@PluginBuilderAttribute
66+
@SuppressWarnings("log4j.public.setter")
67+
private int attributeWithoutPublicSetterButWithSuppressAnnotation;
68+
69+
@PluginElement("layout")
70+
private Layout<? extends Serializable> layout;
71+
72+
@PluginConfiguration
73+
private Configuration config;
74+
75+
@PluginNode
76+
private Node node;
77+
78+
@PluginLoggerContext
79+
private LoggerContext loggerContext;
80+
81+
@PluginValue("value")
82+
private String value;
83+
84+
@Override
85+
public FakePluginPublicSetter build() {
86+
return null;
87+
}
88+
}
89+
}

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/GraalVmProcessorTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ class GraalVmProcessorTest {
6565
"fields",
6666
asList(
6767
asMap("name", "attribute"),
68+
asMap("name", "attributeWithoutPublicSetterButWithSuppressAnnotation"),
6869
asMap("name", "config"),
6970
asMap("name", "layout"),
7071
asMap("name", "loggerContext"),

‎log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/plugins/processor/PluginProcessorPublicSetterTest.java

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.core.config.plugins.processor;
18+
19+
import static java.nio.charset.StandardCharsets.UTF_8;
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
22+
23+
import java.io.File;
24+
import java.nio.file.Path;
25+
import java.nio.file.Paths;
26+
import java.util.Arrays;
27+
import java.util.List;
28+
import java.util.Locale;
29+
import java.util.stream.Collectors;
30+
import javax.tools.Diagnostic;
31+
import javax.tools.DiagnosticCollector;
32+
import javax.tools.JavaCompiler;
33+
import javax.tools.JavaFileObject;
34+
import javax.tools.StandardJavaFileManager;
35+
import javax.tools.ToolProvider;
36+
import org.apache.commons.io.FileUtils;
37+
import org.junit.jupiter.api.AfterEach;
38+
import org.junit.jupiter.api.BeforeEach;
39+
import org.junit.jupiter.api.Test;
40+
41+
public class PluginProcessorPublicSetterTest {
42+
43+
private static final String FAKE_PLUGIN_CLASS_PATH =
44+
"src/test/java/org/apache/logging/log4j/core/config/plugins/processor/" + FakePlugin.class.getSimpleName()
45+
+ "PublicSetter.java.source";
46+
47+
private File createdFile;
48+
private DiagnosticCollector<JavaFileObject> diagnosticCollector;
49+
private List<Diagnostic<? extends JavaFileObject>> errorDiagnostics;
50+
51+
@BeforeEach
52+
void setup() {
53+
// Instantiate the tooling
54+
final JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
55+
diagnosticCollector = new DiagnosticCollector<>();
56+
final StandardJavaFileManager fileManager = compiler.getStandardFileManager(null, Locale.ROOT, UTF_8);
57+
58+
// Get the source files
59+
Path sourceFile = Paths.get(FAKE_PLUGIN_CLASS_PATH);
60+
61+
assertThat(sourceFile).exists();
62+
63+
final File orig = sourceFile.toFile();
64+
createdFile = new File(orig.getParentFile(), "FakePluginPublicSetter.java");
65+
assertDoesNotThrow(() -> FileUtils.copyFile(orig, createdFile));
66+
67+
// get compilation units
68+
Iterable<? extends JavaFileObject> compilationUnits = fileManager.getJavaFileObjects(createdFile);
69+
70+
JavaCompiler.CompilationTask task = compiler.getTask(
71+
null,
72+
fileManager,
73+
diagnosticCollector,
74+
Arrays.asList("-proc:only", "-processor", PluginProcessor.class.getName()),
75+
null,
76+
compilationUnits);
77+
task.call();
78+
79+
errorDiagnostics = diagnosticCollector.getDiagnostics().stream()
80+
.filter(diagnostic -> diagnostic.getKind() == Diagnostic.Kind.ERROR)
81+
.collect(Collectors.toList());
82+
}
83+
84+
@AfterEach
85+
void tearDown() {
86+
assertDoesNotThrow(() -> FileUtils.delete(createdFile));
87+
File pluginsDatFile = Paths.get("Log4j2Plugins.dat").toFile();
88+
if (pluginsDatFile.exists()) {
89+
pluginsDatFile.delete();
90+
}
91+
}
92+
93+
@Test
94+
void warnWhenPluginBuilderAttributeLacksPublicSetter() {
95+
96+
assertThat(errorDiagnostics).anyMatch(errorMessage -> errorMessage
97+
.getMessage(Locale.ROOT)
98+
.contains("The field `attribute` does not have a public setter"));
99+
}
100+
101+
@Test
102+
void ignoreWarningWhenSuppressWarningsIsPresent() {
103+
assertThat(errorDiagnostics)
104+
.allMatch(
105+
errorMessage -> !errorMessage
106+
.getMessage(Locale.ROOT)
107+
.contains(
108+
"The field `attributeWithoutPublicSetterButWithSuppressAnnotation` does not have a public setter"));
109+
}
110+
}

‎log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppender.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public static class Builder<B extends Builder<B>> extends AbstractDatabaseAppend
6060
private ConnectionSource connectionSource;
6161

6262
@PluginBuilderAttribute
63+
@SuppressWarnings("log4j.public.setter")
6364
private boolean immediateFail;
6465

6566
@PluginBuilderAttribute
@@ -80,6 +81,7 @@ public static class Builder<B extends Builder<B>> extends AbstractDatabaseAppend
8081

8182
// TODO Consider moving up to AbstractDatabaseAppender.Builder.
8283
@PluginBuilderAttribute
84+
@SuppressWarnings("log4j.public.setter")
8385
private long reconnectIntervalMillis = DEFAULT_RECONNECT_INTERVAL_MILLIS;
8486

8587
@Override

‎log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/processor/PluginProcessor.java

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,11 @@
2525
import java.io.PrintWriter;
2626
import java.io.StringWriter;
2727
import java.util.ArrayList;
28+
import java.util.Arrays;
2829
import java.util.Collection;
2930
import java.util.Collections;
3031
import java.util.List;
32+
import java.util.Locale;
3133
import java.util.Map;
3234
import java.util.Objects;
3335
import java.util.Set;
@@ -39,14 +41,19 @@
3941
import javax.lang.model.SourceVersion;
4042
import javax.lang.model.element.Element;
4143
import javax.lang.model.element.ElementVisitor;
44+
import javax.lang.model.element.ExecutableElement;
45+
import javax.lang.model.element.Modifier;
4246
import javax.lang.model.element.TypeElement;
47+
import javax.lang.model.element.VariableElement;
4348
import javax.lang.model.util.Elements;
4449
import javax.lang.model.util.SimpleElementVisitor7;
50+
import javax.lang.model.util.Types;
4551
import javax.tools.Diagnostic;
4652
import javax.tools.FileObject;
4753
import javax.tools.StandardLocation;
4854
import org.apache.logging.log4j.core.config.plugins.Plugin;
4955
import org.apache.logging.log4j.core.config.plugins.PluginAliases;
56+
import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute;
5057
import org.apache.logging.log4j.util.Strings;
5158

5259
/**
@@ -60,6 +67,8 @@ public class PluginProcessor extends AbstractProcessor {
6067

6168
private static final Element[] EMPTY_ELEMENT_ARRAY = {};
6269

70+
private static final String SUPPRESS_WARNING_PUBLIC_SETTER_STRING = "log4j.public.setter";
71+
6372
/**
6473
* The location of the plugin cache data file. This file is written to by this processor, and read from by
6574
* {@link org.apache.logging.log4j.core.config.plugins.util.PluginManager}.
@@ -83,6 +92,12 @@ public boolean process(final Set<? extends TypeElement> annotations, final Round
8392
final Set<? extends Element> elements = roundEnv.getElementsAnnotatedWith(Plugin.class);
8493
collectPlugins(elements);
8594
processedElements.addAll(elements);
95+
96+
// process plugin builder Attributes
97+
final Set<? extends Element> pluginAttributeBuilderElements =
98+
roundEnv.getElementsAnnotatedWith(PluginBuilderAttribute.class);
99+
processBuilderAttributes(pluginAttributeBuilderElements);
100+
processedElements.addAll(pluginAttributeBuilderElements);
86101
}
87102
// Write the cache file
88103
if (roundEnv.processingOver() && !processedElements.isEmpty()) {
@@ -107,6 +122,88 @@ public boolean process(final Set<? extends TypeElement> annotations, final Round
107122
return false;
108123
}
109124

125+
private void processBuilderAttributes(final Iterable<? extends Element> elements) {
126+
for (final Element element : elements) {
127+
if (element instanceof VariableElement) {
128+
processBuilderAttributes((VariableElement) element);
129+
}
130+
}
131+
}
132+
133+
private void processBuilderAttributes(final VariableElement element) {
134+
final String fieldName = element.getSimpleName().toString(); // Getting the name of the field
135+
SuppressWarnings suppress = element.getAnnotation(SuppressWarnings.class);
136+
if (suppress != null && Arrays.asList(suppress.value()).contains(SUPPRESS_WARNING_PUBLIC_SETTER_STRING)) {
137+
// Suppress the warning due to annotation
138+
return;
139+
}
140+
final Element enclosingElement = element.getEnclosingElement();
141+
// `element is a field
142+
if (enclosingElement instanceof TypeElement) {
143+
final TypeElement typeElement = (TypeElement) enclosingElement;
144+
// Check the siblings of the field
145+
for (final Element enclosedElement : typeElement.getEnclosedElements()) {
146+
// `enclosedElement is a method or constructor
147+
if (enclosedElement instanceof ExecutableElement) {
148+
final ExecutableElement methodElement = (ExecutableElement) enclosedElement;
149+
final String methodName = methodElement.getSimpleName().toString();
150+
151+
if ((methodName.toLowerCase(Locale.ROOT).startsWith("set") // Typical pattern for setters
152+
|| methodName
153+
.toLowerCase(Locale.ROOT)
154+
.startsWith("with")) // Typical pattern for setters
155+
&& methodElement.getParameters().size()
156+
== 1 // It is a weird pattern to not have public setter
157+
) {
158+
159+
Types typeUtils = processingEnv.getTypeUtils();
160+
161+
boolean followsNamePattern = methodName.equals(
162+
String.format("set%s", expectedFieldNameInASetter(fieldName)))
163+
|| methodName.equals(String.format("with%s", expectedFieldNameInASetter(fieldName)));
164+
165+
// Check if method is public
166+
boolean isPublicMethod = methodElement.getModifiers().contains(Modifier.PUBLIC);
167+
168+
// Check if the return type of the method element is Assignable.
169+
// Assuming it is a builder class the type of it should be assignable to its parent
170+
boolean checkForAssignable = typeUtils.isAssignable(
171+
methodElement.getReturnType(),
172+
methodElement.getEnclosingElement().asType());
173+
174+
boolean foundPublicSetter = followsNamePattern && checkForAssignable && isPublicMethod;
175+
if (foundPublicSetter) {
176+
// Hurray we found a public setter for the field!
177+
return;
178+
}
179+
}
180+
}
181+
}
182+
// If the setter was not found generate a compiler warning.
183+
processingEnv
184+
.getMessager()
185+
.printMessage(
186+
Diagnostic.Kind.ERROR,
187+
String.format(
188+
"The field `%s` does not have a public setter, Note that @SuppressWarnings(\"%s\"), can be used on the field to suppress the compilation error. ",
189+
fieldName, SUPPRESS_WARNING_PUBLIC_SETTER_STRING),
190+
element);
191+
}
192+
}
193+
194+
/**
195+
* Helper method to get the expected Method name in a field.
196+
* For example if the field name is 'isopen', then the expected setter would be 'setOpen' or 'withOpen'
197+
* This method is supposed to return the capitalized 'Open', fieldName which is expected in the setter.
198+
* @param fieldName who's setter we are checking.
199+
* @return The expected fieldName that will come after withxxxx or setxxxx
200+
*/
201+
private static String expectedFieldNameInASetter(String fieldName) {
202+
if (fieldName.startsWith("is")) fieldName = fieldName.substring(2);
203+
204+
return fieldName.isEmpty() ? fieldName : Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1);
205+
}
206+
110207
private void collectPlugins(final Iterable<? extends Element> elements) {
111208
final Elements elementUtils = processingEnv.getElementUtils();
112209
final ElementVisitor<PluginEntry, Plugin> pluginVisitor = new PluginElementVisitor(elementUtils);

‎log4j-core/src/main/java/org/apache/logging/log4j/core/net/SocketPerformancePreferences.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,17 @@ public static SocketPerformancePreferences newBuilder() {
4040

4141
@PluginBuilderAttribute
4242
@Required
43+
@SuppressWarnings("log4j.public.setter")
4344
private int bandwidth;
4445

4546
@PluginBuilderAttribute
4647
@Required
48+
@SuppressWarnings("log4j.public.setter")
4749
private int connectionTime;
4850

4951
@PluginBuilderAttribute
5052
@Required
53+
@SuppressWarnings("log4j.public.setter")
5154
private int latency;
5255

5356
public void apply(final Socket socket) {

0 commit comments

Comments
 (0)