Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: 12950 Enhance error report on ConfigData annotation processing #12960

Merged
merged 5 commits into from
Apr 29, 2024
Merged

fix: 12950 Enhance error report on ConfigData annotation processing #12960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
34a1057
Better error messaging + refactor for future incorporation of differe…
mxtartaglia-sl Apr 23, 2024
5ab42b9
renaming annotation + reorganizing work
mxtartaglia-sl Apr 25, 2024
b09c7de
Merge branch 'develop' into 12950_configdata_processor_error_report
mxtartaglia-sl Apr 26, 2024
7340b9e
Merge branch 'develop' into 12950_configdata_processor_error_report
mxtartaglia-sl Apr 26, 2024
a46752a
Merge branch 'develop' into 12950_configdata_processor_error_report
mxtartaglia-sl Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion hedera-dependency-versions/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ moduleInfo {
version("javax.inject", "1")
version("lazysodium.java", "5.1.1")
version("net.i2p.crypto.eddsa", "0.3.0")
version("org.antlr.antlr4.runtime", "4.11.1")
version("org.antlr.antlr4.runtime", "4.13.1")
version("org.apache.commons.codec", "1.15")
version("org.apache.commons.collections4", "4.4")
version("org.apache.commons.io", "2.15.1")
Expand Down
18 changes: 11 additions & 7 deletions ...g-processor/src/main/java/com/swirlds/config/processor/ConfigDataAnnotationProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public boolean process(
.forEach(typeElement -> handleTypeElement(typeElement, configDocumentationFile));
return true;
} catch (final Exception e) {
log("Error while processing annotations: " + e.getMessage());
log(Kind.ERROR, "Error while processing annotations: " + e.getMessage());
e.printStackTrace();
return false;
}
Expand All @@ -104,12 +104,12 @@ private void handleTypeElement(
final JavaFileObject constantsSourceFile =
getConstantSourceFile(packageName, simpleClassName, typeElement);
log("generating config constants file: " + constantsSourceFile.getName());
ConstantClassFactory.doWork(recordDefinitions.get(0), constantsSourceFile);
ConstantClassFactory.doWork(recordDefinitions.getFirst(), constantsSourceFile);
log("generating config doc file: " + configDocumentationFile.getFileName());
DocumentationFactory.doWork(recordDefinitions.get(0), configDocumentationFile);
DocumentationFactory.doWork(recordDefinitions.getFirst(), configDocumentationFile);
}
} catch (final IOException e) {
throw new RuntimeException("Error while handling " + typeElement, e);
} catch (final Exception e) {
throw new RuntimeException("Error handling " + typeElement, e);
}
}

Expand All @@ -135,11 +135,15 @@ private FileObject getSource(@NonNull final String fileName, @NonNull final Stri
return processingEnv.getFiler().getResource(StandardLocation.SOURCE_PATH, packageName, fileName);
}

protected void log(@NonNull final String message) {
private void log(@NonNull final String message) {
log(Kind.OTHER, message);
}

private void log(@NonNull final Kind kind, @NonNull final String message) {
Objects.requireNonNull(message, "message must not be null");

processingEnv
.getMessager()
.printMessage(Kind.OTHER, ConfigDataAnnotationProcessor.class.getSimpleName() + ": " + message);
.printMessage(kind, ConfigDataAnnotationProcessor.class.getSimpleName() + ": " + message);
}
}
41 changes: 30 additions & 11 deletions ...lds-config-processor/src/main/java/com/swirlds/config/processor/ConstantClassFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.JavaFile;
import com.squareup.javapoet.TypeSpec;
import com.swirlds.config.api.ConfigProperty;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.io.Writer;
Expand All @@ -34,6 +35,12 @@
*/
public final class ConstantClassFactory {

/**
* Property name of:
* {@link ConfigProperty#defaultValue()}
*/
public static final String DEFAULT_VALUE = "defaultValue";

/**
* private constructor to prevent instantiation
*/
Expand Down Expand Up @@ -71,17 +78,29 @@ public static void doWork(
configDataRecordDefinition.propertyDefinitions().forEach(propertyDefinition -> {
final String name = toConstantName(
propertyDefinition.name().replace(configDataRecordDefinition.configDataName() + ".", ""));
FieldSpec fieldSpec = FieldSpec.builder(
String.class, name, Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)
.initializer("$S", propertyDefinition.name())
.addJavadoc(
"Name of the {@link $L#$L} property\n@see $L#$L\n",
originalRecordClassName,
propertyDefinition.fieldName(),
originalRecordClassName,
propertyDefinition.fieldName())
.build();
constantsClassBuilder.addField(fieldSpec);

try {

FieldSpec fieldSpec = FieldSpec.builder(
String.class, name, Modifier.PUBLIC, Modifier.STATIC, Modifier.FINAL)
.initializer("$S", propertyDefinition.name())
.addJavadoc(
"Name of the {@link $L#$L} property\n@see $L#$L\n",
originalRecordClassName,
propertyDefinition.fieldName(),
originalRecordClassName,
propertyDefinition.fieldName())
.build();
constantsClassBuilder.addField(fieldSpec);
} catch (Exception e) {
throw new IllegalArgumentException(
"Error processing record:"
+ configDataRecordDefinition.simpleClassName() + " field:"
+ propertyDefinition.fieldName() + " annotation value:\"" + propertyDefinition.name()
+ "\" cannot be used as a valid constant name. Check if should be a " + DEFAULT_VALUE
+ " instead.",
e);
}
});

TypeSpec constantsClass = constantsClassBuilder.build();
Expand Down
166 changes: 95 additions & 71 deletions ...g-processor/src/main/java/com/swirlds/config/processor/antlr/AntlrConfigRecordParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,94 +25,111 @@
import com.swirlds.config.processor.antlr.generated.JavaParser.RecordComponentContext;
import com.swirlds.config.processor.antlr.generated.JavaParser.RecordDeclarationContext;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.lang.annotation.Annotation;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.antlr.v4.runtime.RuleContext;
import org.antlr.v4.runtime.tree.ParseTree;

public final class AntlrConfigRecordParser {

private static boolean isAnnotatedWithConfigData(
@NonNull final RecordDeclarationContext ctx, @NonNull String packageName, @NonNull List<String> imports) {
/**
* Property name for:
* {@link ConfigProperty#defaultValue()}
*/
private static final String DEFAULT_VALUE = "defaultValue";
/**
* Property name for:
* {@link ConfigProperty#value()}}
*/
private static final String VALUE = "value";

private static boolean isAnnotatedWith(
@NonNull final RecordDeclarationContext ctx,
@NonNull String packageName,
@NonNull List<String> imports,
@NonNull final Class<? extends Annotation> annotation) {
final List<AnnotationContext> allAnnotations = AntlrUtils.getAllAnnotations(ctx);
return AntlrUtils.findAnnotationOfType(ConfigData.class, allAnnotations, packageName, imports)
return AntlrUtils.findAnnotationOfType(annotation, allAnnotations, packageName, imports)
.isPresent();
}

@NonNull
private static Optional<AnnotationContext> getConfigDataAnnotation(
@NonNull final RecordDeclarationContext ctx,
private static Optional<AnnotationContext> getAnnotation(
@NonNull final List<AnnotationContext> annotations,
@NonNull final String packageName,
@NonNull final List<String> imports) {
final List<AnnotationContext> annotations = AntlrUtils.getAllAnnotations(ctx);
return AntlrUtils.findAnnotationOfType(ConfigData.class, annotations, packageName, imports);
@NonNull final List<String> imports,
@NonNull final Class<? extends Annotation> annotation) {
return AntlrUtils.findAnnotationOfType(annotation, annotations, packageName, imports);
}

@NonNull
private static String getConfigDataAnnotationValue(
private static String getAnnotationValue(
@NonNull final RecordDeclarationContext ctx,
@NonNull final String packageName,
@NonNull final List<String> imports) {
return getConfigDataAnnotation(ctx, packageName, imports)
.map(annotationContext -> annotationContext.elementValue())
.map(elementValueContext -> elementValueContext.getText())
@NonNull final List<String> imports,
@NonNull final Class<? extends Annotation> annotation) {
final List<AnnotationContext> annotations = AntlrUtils.getAllAnnotations(ctx);
return getAnnotation(annotations, packageName, imports, annotation)
.map(AnnotationContext::elementValue)
.map(RuleContext::getText)
.map(text -> text.substring(1, text.length() - 1)) // remove quotes
.orElse("");
}

@NonNull
private static Optional<AnnotationContext> getConfigPropertyAnnotation(
private static String getAnnotationPropertyOrElse(
@NonNull final RecordComponentContext ctx,
@NonNull final String packageName,
@NonNull final List<String> imports) {
final List<AnnotationContext> annotations = AntlrUtils.getAllAnnotations(ctx);
return AntlrUtils.findAnnotationOfType(ConfigProperty.class, annotations, packageName, imports);
}

@NonNull
private static String getConfigPropertyAnnotationDefaultValue(
@NonNull final RecordComponentContext ctx,
@NonNull final String packageName,
@NonNull final List<String> imports) {
return getConfigPropertyAnnotation(ctx, packageName, imports)
.flatMap(annotationContext -> AntlrUtils.getAnnotationValue(annotationContext, "defaultValue"))
.orElse(ConfigProperty.UNDEFINED_DEFAULT_VALUE);
}

@NonNull
private static Optional<String> getConfigPropertyAnnotationName(
@NonNull final RecordComponentContext ctx,
@NonNull final String packageName,
@NonNull final List<String> imports) {
return getConfigPropertyAnnotation(ctx, packageName, imports)
.flatMap(annotationContext -> AntlrUtils.getAnnotationValue(annotationContext));
@NonNull final List<String> imports,
@NonNull final Class<? extends Annotation> annotation,
@NonNull final String property,
@NonNull final String orElseValue) {
final List<AnnotationContext> allAnnotations = AntlrUtils.getAllAnnotations(ctx);
return getAnnotation(allAnnotations, packageName, imports, annotation)
.flatMap(annotationContext -> AntlrUtils.getAnnotationValue(annotationContext, property))
.orElse(orElseValue);
}

@NonNull
private static ConfigDataPropertyDefinition createPropertyDefinition(
private static ConfigDataPropertyDefinition createDefinitionFromConfigProperty(
@NonNull final RecordComponentContext ctx,
@NonNull final String configPropertyNamePrefix,
@NonNull final String packageName,
@NonNull final List<String> imports,
@NonNull final Map<String, String> javadocParams) {
final String componentName = ctx.identifier().getText();
final String configPropertyNameSuffix =
getConfigPropertyAnnotationName(ctx, packageName, imports).orElse(componentName);
final String name = createPropertyName(configPropertyNamePrefix, configPropertyNameSuffix);
final String defaultValue = getConfigPropertyAnnotationDefaultValue(ctx, packageName, imports);
final String type = Optional.ofNullable(ctx.typeType().classOrInterfaceType())
.map(c -> c.getText())
.map(typeText -> imports.stream()
.filter(importText -> importText.endsWith(typeText))
.findAny()
.orElse(typeText))
.map(typeText -> getTypeForJavaLang(typeText))
.orElseGet(() -> ctx.typeType().primitiveType().getText());
final String description =
Optional.ofNullable(javadocParams.get(componentName)).orElse("");
return new ConfigDataPropertyDefinition(componentName, name, type, defaultValue, description);
String name = "not-yet-known";
try {
final String configPropertyNameSuffix =
getAnnotationPropertyOrElse(ctx, packageName, imports, ConfigProperty.class, VALUE, componentName);
name = createPropertyName(configPropertyNamePrefix, configPropertyNameSuffix);
final String defaultValue = getAnnotationPropertyOrElse(
ctx,
packageName,
imports,
ConfigProperty.class,
DEFAULT_VALUE,
ConfigProperty.UNDEFINED_DEFAULT_VALUE);
final String type = Optional.ofNullable(ctx.typeType().classOrInterfaceType())
.map(RuleContext::getText)
.map(typeText -> imports.stream()
.filter(importText -> importText.endsWith(typeText))
.findAny()
.orElse(typeText))
.map(AntlrConfigRecordParser::getTypeForJavaLang)
.orElseGet(() -> ctx.typeType().primitiveType().getText());
final String description =
Optional.ofNullable(javadocParams.get(componentName)).orElse("");

return new ConfigDataPropertyDefinition(componentName, name, type, defaultValue, description);
} catch (Exception e) {
throw new IllegalArgumentException(ConfigProperty.class.getTypeName() + " is not correctly defined for "
+ componentName + " property");
}
}

@NonNull
Expand All @@ -139,7 +156,7 @@ private static List<ConfigDataRecordDefinition> createDefinitions(
final String packageName = AntlrUtils.getPackage(unitContext);
final List<String> imports = AntlrUtils.getImports(unitContext);
return AntlrUtils.getRecordDeclarationContext(unitContext).stream()
.filter(c -> isAnnotatedWithConfigData(c, packageName, imports))
.filter(c -> isAnnotatedWith(c, packageName, imports, ConfigData.class))
.map(recordContext -> createDefinition(unitContext, recordContext, packageName, imports))
.collect(Collectors.toList());
}
Expand All @@ -151,22 +168,29 @@ private static ConfigDataRecordDefinition createDefinition(
@NonNull final String packageName,
@NonNull final List<String> imports) {
final String recordName = recordContext.identifier().getText();
final String configPropertyNamePrefix = getConfigDataAnnotationValue(recordContext, packageName, imports);
final Map<String, String> javadocParams = unitContext.children.stream()
.filter(c -> AntlrUtils.isJavaDocNode(c))
.map(c -> c.getText())
.map(t -> AntlrUtils.getJavaDocParams(t))
.reduce((m1, m2) -> {
m1.putAll(m2);
return m1;
})
.orElse(Map.of());
final Set<ConfigDataPropertyDefinition> propertyDefinitions =
recordContext.recordHeader().recordComponentList().recordComponent().stream()
.map(c -> createPropertyDefinition(
c, configPropertyNamePrefix, packageName, imports, javadocParams))
.collect(Collectors.toSet());
return new ConfigDataRecordDefinition(packageName, recordName, configPropertyNamePrefix, propertyDefinitions);

try {
final String configPropertyNamePrefix =
getAnnotationValue(recordContext, packageName, imports, ConfigData.class);
final Map<String, String> javadocParams = unitContext.children.stream()
.filter(AntlrUtils::isJavaDocNode)
.map(ParseTree::getText)
.map(AntlrUtils::getJavaDocParams)
.reduce((m1, m2) -> {
m1.putAll(m2);
return m1;
})
.orElse(Map.of());
final Set<ConfigDataPropertyDefinition> propertyDefinitions =
recordContext.recordHeader().recordComponentList().recordComponent().stream()
.map(c -> createDefinitionFromConfigProperty(
c, configPropertyNamePrefix, packageName, imports, javadocParams))
.collect(Collectors.toSet());
return new ConfigDataRecordDefinition(
packageName, recordName, configPropertyNamePrefix, propertyDefinitions);
} catch (Exception e) {
throw new IllegalArgumentException("Could not process " + packageName + "." + recordName, e);
}
}

/**
Expand All @@ -175,7 +199,7 @@ private static ConfigDataRecordDefinition createDefinition(
* @param fileContent the content of the Java source file
*/
@NonNull
public static List<ConfigDataRecordDefinition> parse(@NonNull final String fileContent) throws IOException {
public static List<ConfigDataRecordDefinition> parse(@NonNull final String fileContent) {
final CompilationUnitContext parsedContext = AntlrUtils.parse(fileContent);
return createDefinitions(parsedContext);
}
Expand Down