Minor code refactoring. Fixed a bunch of Java, PMD, Spotbugs, and Che…

…ckstyle warnings.
david-waltermire · Oct 19, 2024 · c90ae66 · c90ae66
1 parent 1cdc024
commit c90ae66
Show file tree

Hide file tree

Showing 107 changed files with 741 additions and 555 deletions.
diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java
@@ -460,7 +460,7 @@ private String buildHelpFooter() {
       if (subCommands.isEmpty()) {
         retval = "";
       } else {
-        StringBuilder builder = new StringBuilder(64);
+        StringBuilder builder = new StringBuilder(128);
         builder
             .append(System.lineSeparator())
             .append("The following are available commands:")
@@ -553,9 +553,9 @@ protected String buildHelpCliSyntax() {
             builder.append('[');
           }
 
-          builder.append('<');
-          builder.append(argument.getName());
-          builder.append('>');
+          builder.append('<')
+              .append(argument.getName())
+              .append('>');
 
           if (argument.getNumber() > 1) {
             builder.append("...");

diff --git a/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ICommand.java b/cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ICommand.java
@@ -31,7 +31,7 @@ default List<ExtraArgument> getExtraArguments() {
 
   default int requiredExtraArgumentsCount() {
     return (int) getExtraArguments().stream()
-        .filter(arg -> arg.isRequired())
+        .filter(ExtraArgument::isRequired)
         .count();
   }
 

diff --git a/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/ExitCodeTest.java b/cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/ExitCodeTest.java
@@ -20,8 +20,10 @@
 import java.util.List;
 
 /**
+ * Manages the exit code of a command line process.
+ * <p>
  * Logging solution based on
- * https://stackoverflow.com/questions/24205093/how-to-create-a-custom-appender-in-log4j2
+ * https://stackoverflow.com/questions/24205093/how-to-create-a-custom-appender-in-log4j2.
  */
 class ExitCodeTest {
   private static MockedAppender mockedAppender;

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeProvider.java b/core/src/main/java/gov/nist/secauto/metaschema/core/datatype/AbstractDataTypeProvider.java
@@ -11,7 +11,8 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 
@@ -26,15 +27,16 @@ public abstract class AbstractDataTypeProvider implements IDataTypeProvider {
   @NonNull
   private final List<IDataTypeAdapter<?>> library = new LinkedList<>();
   @NonNull
-  protected final Lock instanceLock = new ReentrantLock();
+  private final ReadWriteLock libraryLock = new ReentrantReadWriteLock();
 
   @Override
   public List<? extends IDataTypeAdapter<?>> getJavaTypeAdapters() {
+    Lock readLock = libraryLock.readLock();
     try {
-      instanceLock.lock();
+      readLock.lock();
       return CollectionUtil.unmodifiableList(library);
     } finally {
-      instanceLock.unlock();
+      readLock.unlock();
     }
   }
 
@@ -50,11 +52,12 @@ protected void registerDatatype(@NonNull IDataTypeAdapter<?> adapter) {
     if (adapter.getNames().isEmpty()) {
       throw new IllegalArgumentException("The adapter has no name: " + adapter.getClass().getName());
     }
+    Lock writeLock = libraryLock.writeLock();
     try {
-      instanceLock.lock();
+      writeLock.lock();
       library.add(adapter);
     } finally {
-      instanceLock.unlock();
+      writeLock.unlock();
     }
   }
 }
diff --git a/.../java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/MarkupXmlEventWriter.java b/.../java/gov/nist/secauto/metaschema/core/datatype/markup/flexmark/MarkupXmlEventWriter.java
@@ -31,7 +31,7 @@ public class MarkupXmlEventWriter
     extends AbstractMarkupWriter<XMLEventWriter, XMLStreamException> {
 
   @NonNull
-  protected final XMLEventFactory2 eventFactory;
+  private final XMLEventFactory2 eventFactory;
 
   /**
    * Construct a new event writer.
@@ -78,7 +78,7 @@ protected List<Attribute> handleAttributes(@NonNull Map<String, String> attribut
       attrs = CollectionUtil.emptyList();
     } else {
       attrs = ObjectUtils.notNull(attributes.entrySet().stream()
-          .map((entry) -> eventFactory.createAttribute(entry.getKey(), entry.getValue()))
+          .map(entry -> eventFactory.createAttribute(entry.getKey(), entry.getValue()))
           .collect(Collectors.toList()));
     }
     return attrs;

diff --git a/.../gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/AbstractMarkupWriter.java b/.../gov/nist/secauto/metaschema/core/datatype/markup/flexmark/impl/AbstractMarkupWriter.java
@@ -265,7 +265,6 @@ protected abstract void writeElementStart(
 
   @SuppressWarnings({
       "unchecked",
-      "unused",
       "PMD.UnusedPrivateMethod"
   }) // while unused, keeping code for when inline HTML is supported
   private void writeHtml(Node node) throws E {

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathExpression.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/MetapathExpression.java
@@ -355,7 +355,7 @@ protected <T> T toResultType(@NonNull ISequence<?> sequence, @NonNull ResultType
       throw new InvalidTypeMetapathException(null, String.format("unsupported result type '%s'", resultType.name()));
     }
 
-    return (T) result;
+    return ObjectUtils.asNullableType(result);
   }
 
   /**

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/antlr/ParseTreePrinter.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/antlr/ParseTreePrinter.java
@@ -8,10 +8,16 @@
 import org.antlr.v4.runtime.ParserRuleContext;
 import org.antlr.v4.runtime.RuleContext;
 import org.antlr.v4.runtime.tree.ParseTree;
+import org.eclipse.jdt.annotation.NotOwning;
 
 import java.io.PrintStream;
 
+import edu.umd.cs.findbugs.annotations.NonNull;
+
 public class ParseTreePrinter {
+  @SuppressWarnings("resource")
+  @NotOwning
+  @NonNull
   private final PrintStream outputStream;
   private boolean ignoringWrappers = true;
 
@@ -21,7 +27,7 @@ public class ParseTreePrinter {
    * @param outputStream
    *          the stream to print to
    */
-  public ParseTreePrinter(PrintStream outputStream) {
+  public ParseTreePrinter(@NotOwning @NonNull PrintStream outputStream) {
     this.outputStream = outputStream;
   }
 

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCSTVisitor.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/BuildCSTVisitor.java
@@ -314,10 +314,8 @@ protected IExpression handleArrayConstructor(CurlyarrayconstructorContext ctx) {
   // https://www.w3.org/TR/xpath-31/#id-unary-lookup
   // ===============================================
 
-  @Override
-  protected IExpression handleUnarylookup(UnarylookupContext ctx) {
-    KeyspecifierContext specifier = ctx.keyspecifier();
-
+  @NonNull
+  private IKeySpecifier toKeySpecifier(@NonNull KeyspecifierContext specifier) {
     IKeySpecifier keySpecifier;
     if (specifier.parenthesizedexpr() != null) {
       keySpecifier = AbstractKeySpecifier.newParenthesizedExprKeySpecifier(
@@ -333,7 +331,12 @@ protected IExpression handleUnarylookup(UnarylookupContext ctx) {
     } else {
       throw new UnsupportedOperationException("unknown key specifier");
     }
-    return new UnaryLookup(keySpecifier);
+    return keySpecifier;
+  }
+
+  @Override
+  protected IExpression handleUnarylookup(UnarylookupContext ctx) {
+    return new UnaryLookup(toKeySpecifier(ObjectUtils.requireNonNull(ctx.keyspecifier())));
   }
 
   // =========================================================
@@ -448,9 +451,7 @@ protected Stream<IExpression> parseArgumentList(@NonNull ArgumentlistContext con
       retval = Stream.empty();
     } else {
       retval = context.argument().stream()
-          .map(argument -> {
-            return argument.exprsingle().accept(this);
-          });
+          .map(argument -> argument.exprsingle().accept(this));
     }
     assert retval != null;
 
@@ -540,24 +541,9 @@ protected IExpression handlePostfixexpr(PostfixexprContext context) {
                 left,
                 CollectionUtil.singletonList(parsePredicate((PredicateContext) tree)));
           } else if (tree instanceof LookupContext) {
-            KeyspecifierContext specifier = ((LookupContext) tree).keyspecifier();
-
-            IKeySpecifier keySpecifier;
-            if (specifier.parenthesizedexpr() != null) {
-              keySpecifier = AbstractKeySpecifier.newParenthesizedExprKeySpecifier(
-                  ObjectUtils.requireNonNull(specifier.parenthesizedexpr().accept(this)));
-            } else if (specifier.NCName() != null) {
-              keySpecifier = AbstractKeySpecifier.newNameKeySpecifier(
-                  ObjectUtils.requireNonNull(specifier.NCName().getText()));
-            } else if (specifier.IntegerLiteral() != null) {
-              keySpecifier = AbstractKeySpecifier.newIntegerLiteralKeySpecifier(
-                  IIntegerItem.valueOf(ObjectUtils.requireNonNull(specifier.IntegerLiteral().getText())));
-            } else if (specifier.STAR() != null) {
-              keySpecifier = AbstractKeySpecifier.newWildcardKeySpecifier();
-            } else {
-              throw new UnsupportedOperationException("unknown key specifier");
-            }
-            result = new PostfixLookup(left, keySpecifier);
+            result = new PostfixLookup(
+                left,
+                toKeySpecifier(ObjectUtils.notNull(((LookupContext) tree).keyspecifier())));
           } else {
             result = visit(tree);
           }

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/Wildcard.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/cst/path/Wildcard.java
@@ -53,6 +53,8 @@ public ISequence<? extends INodeItem> accept(
   /**
    * Check the provided items to determine if each item matches the wildcard. All
    * items that match are returned.
+   * <p>
+   * This is an intermediate stream operation.
    *
    * @param <T>
    *          the item Java type
@@ -61,7 +63,7 @@ public ISequence<? extends INodeItem> accept(
    * @return the matching items
    */
   @NonNull
-  public <T extends INodeItem> Stream<T> match(@NonNull Stream<T> items) {
+  public <T extends INodeItem> Stream<T> match(@SuppressWarnings("resource") @NonNull Stream<T> items) {
     Stream<T> nodes = items;
     if (matcher != null) {
       Predicate<IDefinitionNodeItem<?, ?>> test = matcher;

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionLibrary.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionLibrary.java
@@ -10,7 +10,8 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.stream.Stream;
 
 import javax.xml.namespace.QName;
@@ -25,7 +26,7 @@ public class FunctionLibrary implements IFunctionLibrary {
   @NonNull
   private final Map<String, NamedFunctionSet> libraryByName = new HashMap<>(); // NOPMD - intentional
   @NonNull
-  private final Lock instanceLock = new ReentrantLock();
+  private final ReadWriteLock instanceLock = new ReentrantReadWriteLock();
 
   /**
    * Register the provided function signature.
@@ -44,16 +45,17 @@ public final void registerFunction(@NonNull IFunction function) {
   private void registerFunctionByQName(@NonNull IFunction function) {
     QName qname = function.getQName();
     IFunction duplicate;
+    Lock writeLock = instanceLock.writeLock();
     try {
-      instanceLock.lock();
+      writeLock.lock();
       NamedFunctionSet functions = libraryByQName.get(qname);
       if (functions == null) {
         functions = new NamedFunctionSet();
         libraryByQName.put(qname, functions);
       }
       duplicate = functions.addFunction(function);
     } finally {
-      instanceLock.unlock();
+      writeLock.unlock();
     }
     if (duplicate != null) {
       throw new IllegalArgumentException(String.format("Duplicate functions with same arity: %s shadows %s",
@@ -63,8 +65,9 @@ private void registerFunctionByQName(@NonNull IFunction function) {
 
   private void registerFunctionByName(@NonNull IFunction function) {
     String name = function.getName();
+    Lock writeLock = instanceLock.writeLock();
     try {
-      instanceLock.lock();
+      writeLock.lock();
       NamedFunctionSet functions = libraryByName.get(name);
       if (functions == null) {
         functions = new NamedFunctionSet();
@@ -73,46 +76,49 @@ private void registerFunctionByName(@NonNull IFunction function) {
       // replace duplicates
       functions.addFunction(function);
     } finally {
-      instanceLock.unlock();
+      writeLock.unlock();
     }
   }
 
   @Override
   public Stream<IFunction> stream() {
+    Lock readLock = instanceLock.readLock();
     try {
-      instanceLock.lock();
+      readLock.lock();
       return ObjectUtils.notNull(libraryByQName.values().stream().flatMap(NamedFunctionSet::getFunctionsAsStream));
     } finally {
-      instanceLock.unlock();
+      readLock.unlock();
     }
   }
 
   @Override
   public IFunction getFunction(@NonNull String name, int arity) {
     IFunction retval = null;
+    Lock readLock = instanceLock.readLock();
     try {
-      instanceLock.lock();
+      readLock.lock();
       NamedFunctionSet functions = libraryByName.get(name);
       if (functions != null) {
         retval = functions.getFunctionWithArity(arity);
       }
     } finally {
-      instanceLock.unlock();
+      readLock.unlock();
     }
     return retval;
   }
 
   @Override
   public IFunction getFunction(@NonNull QName name, int arity) {
     IFunction retval = null;
+    Lock readLock = instanceLock.readLock();
     try {
-      instanceLock.lock();
+      readLock.lock();
       NamedFunctionSet functions = libraryByQName.get(name);
       if (functions != null) {
         retval = functions.getFunctionWithArity(arity);
       }
     } finally {
-      instanceLock.unlock();
+      readLock.unlock();
     }
     return retval;
   }

diff --git a/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionService.java b/core/src/main/java/gov/nist/secauto/metaschema/core/metapath/function/FunctionService.java
@@ -10,8 +10,6 @@
 
 import java.util.ServiceLoader;
 import java.util.ServiceLoader.Provider;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReentrantLock;
 import java.util.stream.Stream;
 
 import javax.xml.namespace.QName;
@@ -20,13 +18,11 @@
 import nl.talsmasoftware.lazy4j.Lazy;
 
 public final class FunctionService {
-  private static final Lazy<FunctionService> INSTANCE = Lazy.lazy(() -> new FunctionService());
+  private static final Lazy<FunctionService> INSTANCE = Lazy.lazy(FunctionService::new);
   @NonNull
   private final ServiceLoader<IFunctionLibrary> loader;
   @NonNull
   private final Lazy<IFunctionLibrary> library;
-  @NonNull
-  private final Lock instanceLock = new ReentrantLock();
 
   /**
    * Get the singleton instance of the function service.
@@ -93,13 +89,7 @@ public Stream<IFunction> stream() {
    *           if a matching function was not found
    */
   public IFunction getFunction(@NonNull String name, int arity) {
-    IFunction retval;
-    try {
-      instanceLock.lock();
-      retval = getLibrary().getFunction(name, arity);
-    } finally {
-      instanceLock.unlock();
-    }
+    IFunction retval = getLibrary().getFunction(name, arity);
 
     if (retval == null) {
       throw new StaticMetapathException(StaticMetapathException.NO_FUNCTION_MATCH,
@@ -122,13 +112,7 @@ public IFunction getFunction(@NonNull String name, int arity) {
    *           if a matching function was not found
    */
   public IFunction getFunction(@NonNull QName name, int arity) {
-    IFunction retval;
-    try {
-      instanceLock.lock();
-      retval = getLibrary().getFunction(name, arity);
-    } finally {
-      instanceLock.unlock();
-    }
+    IFunction retval = getLibrary().getFunction(name, arity);
 
     if (retval == null) {
       throw new StaticMetapathException(StaticMetapathException.NO_FUNCTION_MATCH,
-Original file line number
+Diff line change
@@ Expand Up @@
           throw new InvalidTypeMetapathException(null, String.format("unsupported result type '%s'", resultType.name()));
         }
-        return (T) result;
+        return ObjectUtils.asNullableType(result);
       }
       /**
@@ Expand Down @@