Skip to content

Commit

Permalink
fix: 12950 Enhance error report on ConfigData annotation processing (#…
Browse files Browse the repository at this point in the history 
…12960)

Signed-off-by: mxtartaglia <maxi@swirldslabs.com>
Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>
  • Loading branch information
@mxtartaglia-sl @stoyanov-st
mxtartaglia-sl authored and stoyanov-st committed May 15, 2024
1 parent fda8623 commit 3a36b72
Show file tree
Hide file tree
Showing 5 changed files with 162 additions and 117 deletions.
2 changes: 1 addition & 1 deletion hedera-dependency-versions/build.gradle.kts
View file
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
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
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
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

0 comments on commit 3a36b72

Please sign in to comment.