Skip to content

Commit

Permalink
Code cleanup and addressing PMD and Spotbug warnings.
Browse files Browse the repository at this point in the history
  • Loading branch information
david-waltermire committed Oct 31, 2024
1 parent 831de55 commit 76839ab
Show file tree
Hide file tree
Showing 32 changed files with 244 additions and 292 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ public Object[] toArray() {
}

@Override
public <T> T[] toArray(T[] a) {
return getValue().toArray(a);
public <T> T[] toArray(T[] array) {
return getValue().toArray(array);
}

@SuppressWarnings("PMD.NullAssignment")
@Override
public List<ITEM> getValue() {
instanceLock.lock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ class BuildCstVisitorTest {
@RegisterExtension
Mockery context = new JUnit5Mockery();

@SuppressWarnings("null")
@NonNull
private IDocumentNodeItem newTestDocument() {
private static IDocumentNodeItem newTestDocument() {
MockNodeItemFactory factory = new MockNodeItemFactory();

return factory.document(URI.create("http://example.com/content"), ROOT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ void testFlagAxis() {

Assertions.assertThat(actual.getValue())
.hasOnlyElementsOfType(IFlagNodeItem.class)
.map(flag -> FnData.fnDataItem(flag).asString())
.map(flag -> FnData.fnDataItem(ObjectUtils.requireNonNull(flag)).asString())
.containsExactly("flag-2-b-v1", "flag-2-b-v2", "flag-2-b-v3");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@
@SuppressWarnings("checkstyle:MissingJavadocMethodCheck")
@SuppressFBWarnings("RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT")
public class MockNodeItemFactory {
@SuppressWarnings("exports")
public MockNodeItemFactory() {
}

@SuppressWarnings("null")
@NonNull
protected <T extends INodeItem> T newMock(@NonNull Class<T> clazz, @NonNull String name) {
Expand All @@ -44,7 +40,7 @@ protected <T extends INodeItem> T newMock(@NonNull Class<T> clazz, @NonNull Stri
.append('-')
.append(UUID.randomUUID().toString())
.toString();
return Mockito.mock(clazz, withSettings().defaultAnswer(Answers.CALLS_REAL_METHODS));
return Mockito.mock(clazz, withSettings().name(mockName).defaultAnswer(Answers.CALLS_REAL_METHODS));
}

@NonNull
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
import gov.nist.secauto.metaschema.core.model.ISource;
import gov.nist.secauto.metaschema.core.util.CollectionUtil;

import org.jmock.api.Invocation;
import org.jmock.lib.action.CustomAction;
import org.junit.jupiter.api.Test;

import java.net.URI;
Expand Down Expand Up @@ -70,8 +68,6 @@ void testAllowedValuesAllowOther() {
.allowsOther(true)
.build();

DynamicContext dynamicContext = new DynamicContext();

doReturn(flagDefinition).when(flag).getDefinition();
doReturn("flag/path").when(flag).toPath(any(IPathFormatter.class));

Expand All @@ -85,6 +81,7 @@ void testAllowedValuesAllowOther() {

FindingCollectingConstraintValidationHandler handler = new FindingCollectingConstraintValidationHandler();
DefaultConstraintValidator validator = new DefaultConstraintValidator(handler);
DynamicContext dynamicContext = new DynamicContext();
validator.validate(flag, dynamicContext);
validator.finalizeValidation(dynamicContext);

Expand Down Expand Up @@ -121,8 +118,6 @@ void testAllowedValuesMultipleAllowOther() {
List<? extends IAllowedValuesConstraint> allowedValuesConstraints
= List.of(allowedValues1, allowedValues2);

DynamicContext dynamicContext = new DynamicContext();

doReturn(flagDefinition).when(flag).getDefinition();
doReturn("flag/path").when(flag).toPath(any(IPathFormatter.class));

Expand All @@ -136,6 +131,7 @@ void testAllowedValuesMultipleAllowOther() {

FindingCollectingConstraintValidationHandler handler = new FindingCollectingConstraintValidationHandler();
DefaultConstraintValidator validator = new DefaultConstraintValidator(handler);
DynamicContext dynamicContext = new DynamicContext();
validator.validate(flag, dynamicContext);
validator.finalizeValidation(dynamicContext);

Expand Down Expand Up @@ -206,23 +202,4 @@ void testMultipleAllowedValuesConflictingAllowOther() {
() -> assertThat("only 1 finding", handler.getFindings(), hasSize(1)),
() -> assertThat("finding is for a flag node", handler.getFindings(), hasItem(hasProperty("node", is(flag1)))));
}

private static class FlagVisitorAction
extends CustomAction {
@NonNull
private DynamicContext dynamicContext;

public FlagVisitorAction(@NonNull DynamicContext dynamicContext) {
super("return the flag");
this.dynamicContext = dynamicContext;
}

@Override
public Object invoke(Invocation invocation) {
IFlagNodeItem thisFlag = (IFlagNodeItem) invocation.getInvokedObject();
assert thisFlag != null;
DefaultConstraintValidator.Visitor visitor = (DefaultConstraintValidator.Visitor) invocation.getParameter(0);
return visitor.visitFlag(thisFlag, dynamicContext);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public IBoundModule registerModule(
}

@NonNull
protected abstract Class<? extends IBoundModule> handleUnboundModule(IModule key);
protected abstract Class<? extends IBoundModule> handleUnboundModule(@NonNull IModule key);

/**
* Get the Module instance for a given class annotated by the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public IBoundModule loadModule(Class<? extends IBoundModule> clazz, IBindingCont

@Override
public void postProcessModule(IModule module, IBindingContext bindingContext) {
processModule(module, bindingContext);
processModule(module);
delegate.postProcessModule(module, bindingContext);
}

Expand All @@ -68,7 +68,7 @@ public IBoundModule registerModule(IModule module, IBindingContext bindingContex
postProcessedModulesLock.lock();
try {
// process before registering
processModule(module, bindingContext);
processModule(module);

boundModule = delegate.registerModule(module, bindingContext);

Expand All @@ -80,7 +80,7 @@ public IBoundModule registerModule(IModule module, IBindingContext bindingContex
return boundModule;
}

protected void processModule(IModule module, IBindingContext bindingContext) {
private void processModule(@NonNull IModule module) {
postProcessedModulesLock.lock();
try {
if (!postProcessedModules.contains(module)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,9 @@ public static IProduction compileModule(
IProduction production = JavaGenerator.generate(module, classDir, bindingConfiguration);
List<IGeneratedClass> classesToCompile = production.getGeneratedClasses().collect(Collectors.toList());

List<Path> classes = classesToCompile.stream()
List<Path> classes = ObjectUtils.notNull(classesToCompile.stream()
.map(IGeneratedClass::getClassFile)
.collect(Collectors.toUnmodifiableList());
.collect(Collectors.toUnmodifiableList()));

JavaCompilerSupport compiler = new JavaCompilerSupport(classDir);

Expand All @@ -128,7 +128,7 @@ public static IProduction compileModule(
ModuleDescriptor descriptor = databindModule.getDescriptor();
if (descriptor != null) {
// add the databind module to the task
compiler.addRootModule(descriptor.name());
compiler.addRootModule(ObjectUtils.notNull(descriptor.name()));
usingModule = true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ public IGeneratedClass generatePackageInfoClass(
}

/**
* Creates and configures a builder, for a Module module, that can be used to
* generate a Java class.
* Creates and configures a builder for a module that can be used to generate a
* Java class.
*
* @param module
* a parsed Module module
Expand All @@ -251,51 +251,7 @@ protected TypeSpec.Builder newClassBuilder(
.addModifiers(Modifier.FINAL);

builder.superclass(AbstractBoundModule.class);

AnnotationSpec.Builder moduleAnnotation = AnnotationSpec.builder(MetaschemaModule.class);

ITypeResolver typeResolver = getTypeResolver();
for (IFieldDefinition definition : module.getFieldDefinitions()) {
if (definition.hasChildren()) {
moduleAnnotation.addMember("fields", "$T.class", typeResolver.getClassName(definition));
}
}

for (IAssemblyDefinition definition : module.getAssemblyDefinitions()) {
moduleAnnotation.addMember(
"assemblies",
"$T.class",
typeResolver.getClassName(ObjectUtils.notNull(definition)));
}

for (IModule moduleImport : module.getImportedModules()) {
moduleAnnotation.addMember(
"imports",
"$T.class",
typeResolver.getClassName(ObjectUtils.notNull(moduleImport)));
}

Map<String, String> bindings = module.getNamespaceBindings();
if (!bindings.isEmpty()) {
for (Map.Entry<String, String> binding : bindings.entrySet()) {
moduleAnnotation.addMember(
"nsBindings",
"$L",
AnnotationSpec.builder(NsBinding.class)
.addMember("prefix", "$S", binding.getKey())
.addMember("uri", "$S", binding.getValue())
.build());
}
}

{
MarkupMultiline remarks = module.getRemarks();
if (remarks != null) {
moduleAnnotation.addMember("remarks", "$S", remarks.toMarkdown());
}
}

builder.addAnnotation(moduleAnnotation.build());
builder.addAnnotation(buildModuleAnnotation(module).build());

builder.addField(
FieldSpec.builder(MarkupLine.class, "NAME", Modifier.PRIVATE, Modifier.STATIC, Modifier.FINAL)
Expand Down Expand Up @@ -477,6 +433,50 @@ protected TypeSpec.Builder newClassBuilder(
return ObjectUtils.notNull(builder);
}

private AnnotationSpec.Builder buildModuleAnnotation(@NonNull IModule module) {
AnnotationSpec.Builder retval = AnnotationSpec.builder(MetaschemaModule.class);

ITypeResolver typeResolver = getTypeResolver();
for (IFieldDefinition definition : module.getFieldDefinitions()) {
if (definition.hasChildren()) {
retval.addMember("fields", "$T.class", typeResolver.getClassName(definition));
}
}

for (IAssemblyDefinition definition : module.getAssemblyDefinitions()) {
retval.addMember(
"assemblies",
"$T.class",
typeResolver.getClassName(ObjectUtils.notNull(definition)));
}

for (IModule moduleImport : module.getImportedModules()) {
retval.addMember(
"imports",
"$T.class",
typeResolver.getClassName(ObjectUtils.notNull(moduleImport)));
}

Map<String, String> bindings = module.getNamespaceBindings();
if (!bindings.isEmpty()) {
for (Map.Entry<String, String> binding : bindings.entrySet()) {
retval.addMember(
"nsBindings",
"$L",
AnnotationSpec.builder(NsBinding.class)
.addMember("prefix", "$S", binding.getKey())
.addMember("uri", "$S", binding.getValue())
.build());
}
}

MarkupMultiline remarks = module.getRemarks();
if (remarks != null) {
retval.addMember("remarks", "$S", remarks.toMarkdown());
}
return retval;
}

/**
* Generate the contents of the class represented by the provided
* {@code builder}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,29 @@ public Set<IModelDefinition> buildField(
FieldSpec.Builder fieldBuilder) {
super.buildField(typeBuilder, fieldBuilder);

AnnotationSpec.Builder annotation = AnnotationSpec.builder(BoundFlag.class);

IFlagInstance instance = getInstance();

fieldBuilder.addAnnotation(buildBoundFlagAnnotation(instance).build());

IModelDefinition parent = instance.getContainingDefinition();
IFlagInstance jsonKey = parent.getJsonKey();
if (instance.equals(jsonKey)) {
fieldBuilder.addAnnotation(JsonKey.class);
}

if (parent instanceof IFieldDefinition) {
IFieldDefinition parentField = (IFieldDefinition) parent;

if (parentField.hasJsonValueKeyFlagInstance() && instance.equals(parentField.getJsonValueKeyFlagInstance())) {
fieldBuilder.addAnnotation(JsonFieldValueKeyFlag.class);
}
}
return CollectionUtil.emptySet();
}

private static AnnotationSpec.Builder buildBoundFlagAnnotation(@NonNull IFlagInstance instance) {
AnnotationSpec.Builder annotation = AnnotationSpec.builder(BoundFlag.class);

String formalName = instance.getEffectiveFormalName();
if (formalName != null) {
annotation.addMember("formalName", "$S", formalName);
Expand Down Expand Up @@ -97,21 +116,6 @@ public Set<IModelDefinition> buildField(

AnnotationGenerator.buildValueConstraints(annotation, definition);

fieldBuilder.addAnnotation(annotation.build());

IModelDefinition parent = instance.getContainingDefinition();
IFlagInstance jsonKey = parent.getJsonKey();
if (instance.equals(jsonKey)) {
fieldBuilder.addAnnotation(JsonKey.class);
}

if (parent instanceof IFieldDefinition) {
IFieldDefinition parentField = (IFieldDefinition) parent;

if (parentField.hasJsonValueKeyFlagInstance() && instance.equals(parentField.getJsonValueKeyFlagInstance())) {
fieldBuilder.addAnnotation(JsonFieldValueKeyFlag.class);
}
}
return CollectionUtil.emptySet();
return annotation;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ private IBoundObject readComplexDefinitionObject(
IBoundObject item = definition.newInstance(
JsonLocation.NA.equals(location)
? null
: () -> new MetaschemaData(location));
: () -> new MetaschemaData(ObjectUtils.requireNonNull(location)));

try {
// call pre-parse initialization hook
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,9 @@ public <CLASS> CLASS read(@NonNull IBoundDefinitionModelComplex definition) thro
ItemReadHandler handler = new ItemReadHandler(ObjectUtils.notNull(event.asStartElement()));
Object value = definition.readItem(null, handler);
if (value == null) {
event = reader.peek();
throw new IOException(String.format("Unable to read data%s",
XmlEventUtil.generateLocationMessage(reader.peek())));
event == null ? "" : XmlEventUtil.generateLocationMessage(event)));
}

return ObjectUtils.asType(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import gov.nist.secauto.metaschema.core.datatype.IDataTypeAdapter;
import gov.nist.secauto.metaschema.core.model.IBoundObject;
import gov.nist.secauto.metaschema.core.model.IFieldInstanceAbsolute;
import gov.nist.secauto.metaschema.core.util.ObjectUtils;
import gov.nist.secauto.metaschema.databind.IBindingContext;
import gov.nist.secauto.metaschema.databind.model.impl.DefinitionField;
import gov.nist.secauto.metaschema.databind.model.impl.InstanceModelFieldComplex;
Expand Down Expand Up @@ -42,8 +43,8 @@ static IBoundInstanceModelField<?> newInstance(
IBoundInstanceModelField<?> retval;
if (IBoundObject.class.isAssignableFrom(itemType)) {
IBindingContext bindingContext = containingDefinition.getBindingContext();
IBoundDefinitionModel<?> definition
= bindingContext.getBoundDefinitionForClass(itemType.asSubclass(IBoundObject.class));
IBoundDefinitionModel<?> definition = bindingContext.getBoundDefinitionForClass(
ObjectUtils.notNull(itemType.asSubclass(IBoundObject.class)));
if (definition == null) {
throw new IllegalStateException(String.format(
"The field '%s' on class '%s' is not bound to a Metaschema field",
Expand Down
Loading

0 comments on commit 76839ab

Please sign in to comment.