From 0e640b74e0af270817023bbc560d179580b5f646 Mon Sep 17 00:00:00 2001 From: "Philip K. Warren" Date: Wed, 11 Jun 2025 17:24:52 -0500 Subject: [PATCH 1/3] Reduce visibility of previously internal package As part of refactoring, the previous internal package (where the validator implementation is found) was refactored to the build.buf.protovalidate package. As part of this change, we should ensure that all previously internal classes are final (to prevent subclassing) and that methods not overridden from an interface use package private or lower visibility. --- .../build/buf/protovalidate/AnyEvaluator.java | 2 +- .../buf/protovalidate/AstExpression.java | 8 ++-- .../build/buf/protovalidate/CelPrograms.java | 2 +- .../buf/protovalidate/CompiledProgram.java | 6 +-- .../buf/protovalidate/CustomOverload.java | 19 +++++---- .../buf/protovalidate/DescriptorMappings.java | 4 +- .../EmbeddedMessageEvaluator.java | 2 +- .../buf/protovalidate/EnumEvaluator.java | 2 +- .../buf/protovalidate/EvaluatorBuilder.java | 5 +-- .../build/buf/protovalidate/Expression.java | 10 ++--- .../buf/protovalidate/FieldEvaluator.java | 4 +- .../buf/protovalidate/FieldPathUtils.java | 4 +- .../java/build/buf/protovalidate/Format.java | 2 +- .../buf/protovalidate/ListEvaluator.java | 2 +- .../build/buf/protovalidate/MapEvaluator.java | 6 +-- .../buf/protovalidate/MessageEvaluator.java | 4 +- .../protovalidate/MessageOneofEvaluator.java | 6 +-- .../build/buf/protovalidate/MessageValue.java | 2 +- .../build/buf/protovalidate/NowVariable.java | 6 +-- .../build/buf/protovalidate/ObjectValue.java | 2 +- .../buf/protovalidate/OneofEvaluator.java | 4 +- .../build/buf/protovalidate/ProtoAdapter.java | 4 +- .../build/buf/protovalidate/RuleCache.java | 14 +++---- .../build/buf/protovalidate/RuleResolver.java | 2 +- .../buf/protovalidate/RuleViolation.java | 40 +++++++++---------- .../protovalidate/RuleViolationHelper.java | 7 +--- .../UnknownDescriptorEvaluator.java | 2 +- .../java/build/buf/protovalidate/Uri.java | 6 +-- .../buf/protovalidate/ValidateLibrary.java | 4 +- .../buf/protovalidate/ValidatorImpl.java | 3 +- .../buf/protovalidate/ValueEvaluator.java | 12 +++--- .../build/buf/protovalidate/Variable.java | 17 ++++---- .../build/buf/protovalidate/Violation.java | 4 +- 33 files changed, 102 insertions(+), 115 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/AnyEvaluator.java b/src/main/java/build/buf/protovalidate/AnyEvaluator.java index a6b782ae..cbcd1e63 100644 --- a/src/main/java/build/buf/protovalidate/AnyEvaluator.java +++ b/src/main/java/build/buf/protovalidate/AnyEvaluator.java @@ -32,7 +32,7 @@ * com.google.protobuf.Any}'s within an expression, breaking evaluation if the type is unknown at * runtime. */ -class AnyEvaluator implements Evaluator { +final class AnyEvaluator implements Evaluator { private final RuleViolationHelper helper; private final Descriptors.FieldDescriptor typeURLDescriptor; private final Set in; diff --git a/src/main/java/build/buf/protovalidate/AstExpression.java b/src/main/java/build/buf/protovalidate/AstExpression.java index 81841d9b..1fcd3ebb 100644 --- a/src/main/java/build/buf/protovalidate/AstExpression.java +++ b/src/main/java/build/buf/protovalidate/AstExpression.java @@ -22,12 +22,12 @@ import dev.cel.compiler.CelCompiler; /** {@link AstExpression} is a compiled CEL {@link CelAbstractSyntaxTree}. */ -class AstExpression { +final class AstExpression { /** The compiled CEL AST. */ - public final CelAbstractSyntaxTree ast; + final CelAbstractSyntaxTree ast; /** Contains the original expression from the proto file. */ - public final Expression source; + final Expression source; /** Constructs a new {@link AstExpression}. */ private AstExpression(CelAbstractSyntaxTree ast, Expression source) { @@ -43,7 +43,7 @@ private AstExpression(CelAbstractSyntaxTree ast, Expression source) { * @return The compiled {@link AstExpression}. * @throws CompilationException if the expression compilation fails. */ - public static AstExpression newAstExpression(CelCompiler cel, Expression expr) + static AstExpression newAstExpression(CelCompiler cel, Expression expr) throws CompilationException { CelValidationResult compileResult = cel.compile(expr.expression); if (!compileResult.getAllIssues().isEmpty()) { diff --git a/src/main/java/build/buf/protovalidate/CelPrograms.java b/src/main/java/build/buf/protovalidate/CelPrograms.java index 0921642f..6dbd25f1 100644 --- a/src/main/java/build/buf/protovalidate/CelPrograms.java +++ b/src/main/java/build/buf/protovalidate/CelPrograms.java @@ -21,7 +21,7 @@ import org.jspecify.annotations.Nullable; /** Evaluator that executes a {@link CompiledProgram}. */ -class CelPrograms implements Evaluator { +final class CelPrograms implements Evaluator { private final RuleViolationHelper helper; /** A list of {@link CompiledProgram} that will be executed against the input message. */ diff --git a/src/main/java/build/buf/protovalidate/CompiledProgram.java b/src/main/java/build/buf/protovalidate/CompiledProgram.java index 27ca0a11..ae7e3e94 100644 --- a/src/main/java/build/buf/protovalidate/CompiledProgram.java +++ b/src/main/java/build/buf/protovalidate/CompiledProgram.java @@ -25,7 +25,7 @@ * {@link CompiledProgram} is a parsed and type-checked {@link Program} along with the source {@link * Expression}. */ -class CompiledProgram { +final class CompiledProgram { /** A compiled CEL program that can be evaluated against a set of variable bindings. */ private final Program program; @@ -52,7 +52,7 @@ class CompiledProgram { * @param rulePath The field path from the FieldRules to the rule value. * @param ruleValue The rule value. */ - public CompiledProgram( + CompiledProgram( Program program, Expression source, @Nullable FieldPath rulePath, @@ -74,7 +74,7 @@ public CompiledProgram( * violations. * @throws ExecutionException If the evaluation of the CEL program fails with an error. */ - public RuleViolation.@Nullable Builder eval(Value fieldValue, CelVariableResolver variables) + RuleViolation.@Nullable Builder eval(Value fieldValue, CelVariableResolver variables) throws ExecutionException { Object value; try { diff --git a/src/main/java/build/buf/protovalidate/CustomOverload.java b/src/main/java/build/buf/protovalidate/CustomOverload.java index 28d468a9..96ebc39c 100644 --- a/src/main/java/build/buf/protovalidate/CustomOverload.java +++ b/src/main/java/build/buf/protovalidate/CustomOverload.java @@ -74,10 +74,11 @@ static List create() { } /** - * This implementes that standard {@code bytes_to_string} function. We override it because the CEL + * This implements that standard {@code bytes_to_string} function. We override it because the CEL * library doesn't validate that the bytes are valid utf-8. * - *

Workround until https://github.com/google/cel-java/pull/717 lands. + *

Workaround until cel-java issue + * 717 lands. */ private static CelFunctionBinding celBytesToString() { return CelFunctionBinding.from( @@ -390,9 +391,9 @@ private static CelFunctionBinding celIsHostAndPort() { *

The host can be one of: * *

    - *
  • An IPv4 address in dotted decimal format, for example "192.168.0.1". - *
  • An IPv6 address enclosed in square brackets, for example "[::1]". - *
  • A hostname, for example "example.com". + *
  • An IPv4 address in dotted decimal format, for example {@code 192.168.0.1}. + *
  • An IPv6 address enclosed in square brackets, for example {@code [::1]}. + *
  • A hostname, for example {@code example.com}. *
* *

The port is separated by a colon. It must be non-empty, with a decimal number in the range @@ -570,7 +571,8 @@ static boolean isIp(String addr, long ver) { } /** - * Returns true if the string is a URI, for example "https://example.com/foo/bar?baz=quux#frag". + * Returns true if the string is a URI, for example {@code + * https://example.com/foo/bar?baz=quux#frag}. * *

URI is defined in the internet standard RFC 3986. Zone Identifiers in IPv6 address literals * are supported (RFC 6874). @@ -580,8 +582,9 @@ private static boolean isUri(String str) { } /** - * Returns true if the string is a URI Reference - a URI such as - * "https://example.com/foo/bar?baz=quux#frag", or a Relative Reference such as "./foo/bar?query". + * Returns true if the string is a URI Reference - a URI such as {@code + * https://example.com/foo/bar?baz=quux#frag}, or a Relative Reference such as {@code + * ./foo/bar?query}. * *

URI, URI Reference, and Relative Reference are defined in the internet standard RFC 3986. * Zone Identifiers in IPv6 address literals are supported (RFC 6874). diff --git a/src/main/java/build/buf/protovalidate/DescriptorMappings.java b/src/main/java/build/buf/protovalidate/DescriptorMappings.java index dbbe42a6..0a6758fd 100644 --- a/src/main/java/build/buf/protovalidate/DescriptorMappings.java +++ b/src/main/java/build/buf/protovalidate/DescriptorMappings.java @@ -105,7 +105,7 @@ private DescriptorMappings() {} * @return The rules field descriptor for the specified wrapper fully qualified name. */ @Nullable - public static FieldDescriptor expectedWrapperRules(String fqn) { + static FieldDescriptor expectedWrapperRules(String fqn) { switch (fqn) { case "google.protobuf.BoolValue": return EXPECTED_STANDARD_RULES.get(FieldDescriptor.Type.BOOL); @@ -136,7 +136,7 @@ public static FieldDescriptor expectedWrapperRules(String fqn) { * @param kind The protobuf field type. * @return The corresponding CEL type for the protobuf field. */ - public static CelType protoKindToCELType(FieldDescriptor.Type kind) { + static CelType protoKindToCELType(FieldDescriptor.Type kind) { switch (kind) { case FLOAT: case DOUBLE: diff --git a/src/main/java/build/buf/protovalidate/EmbeddedMessageEvaluator.java b/src/main/java/build/buf/protovalidate/EmbeddedMessageEvaluator.java index 35b005a7..ea0214f8 100644 --- a/src/main/java/build/buf/protovalidate/EmbeddedMessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/EmbeddedMessageEvaluator.java @@ -18,7 +18,7 @@ import java.util.Collections; import java.util.List; -class EmbeddedMessageEvaluator implements Evaluator { +final class EmbeddedMessageEvaluator implements Evaluator { private final RuleViolationHelper helper; private final MessageEvaluator messageEvaluator; diff --git a/src/main/java/build/buf/protovalidate/EnumEvaluator.java b/src/main/java/build/buf/protovalidate/EnumEvaluator.java index e9a0c506..6167f159 100644 --- a/src/main/java/build/buf/protovalidate/EnumEvaluator.java +++ b/src/main/java/build/buf/protovalidate/EnumEvaluator.java @@ -28,7 +28,7 @@ * {@link EnumEvaluator} checks an enum value being a member of the defined values exclusively. This * check is handled outside CEL as enums are completely type erased to integers. */ -class EnumEvaluator implements Evaluator { +final class EnumEvaluator implements Evaluator { private final RuleViolationHelper helper; /** Captures all the defined values for this enum */ diff --git a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java index 5cc590b9..2453b53e 100644 --- a/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java +++ b/src/main/java/build/buf/protovalidate/EvaluatorBuilder.java @@ -43,7 +43,7 @@ import org.jspecify.annotations.Nullable; /** A build-through cache of message evaluators keyed off the provided descriptor. */ -class EvaluatorBuilder { +final class EvaluatorBuilder { private static final FieldPathElement CEL_FIELD_PATH_ELEMENT = FieldPathUtils.fieldPathElement( FieldRules.getDescriptor().findFieldByNumber(FieldRules.CEL_FIELD_NUMBER)); @@ -150,8 +150,7 @@ private DescriptorCacheBuilder( * @return Unmodifiable map of descriptors to evaluators. * @throws CompilationException If an error occurs compiling a rule on the cache. */ - public Map build(Descriptor descriptor) - throws CompilationException { + Map build(Descriptor descriptor) throws CompilationException { createMessageEvaluator(descriptor); return Collections.unmodifiableMap(cache); } diff --git a/src/main/java/build/buf/protovalidate/Expression.java b/src/main/java/build/buf/protovalidate/Expression.java index ce5a33ad..f5e02e6c 100644 --- a/src/main/java/build/buf/protovalidate/Expression.java +++ b/src/main/java/build/buf/protovalidate/Expression.java @@ -19,15 +19,15 @@ import java.util.List; /** Expression represents a single CEL expression. */ -class Expression { +final class Expression { /** The id of the rule. */ - public final String id; + final String id; /** The message of the rule. */ - public final String message; + final String message; /** The expression of the rule. */ - public final String expression; + final String expression; /** * Constructs a new Expression. @@ -57,7 +57,7 @@ private Expression(Rule rule) { * @param rules The list of rules. * @return The list of expressions. */ - public static List fromRules(List rules) { + static List fromRules(List rules) { List expressions = new ArrayList<>(); for (build.buf.validate.Rule rule : rules) { expressions.add(new Expression(rule)); diff --git a/src/main/java/build/buf/protovalidate/FieldEvaluator.java b/src/main/java/build/buf/protovalidate/FieldEvaluator.java index a8366447..4dfcfb6f 100644 --- a/src/main/java/build/buf/protovalidate/FieldEvaluator.java +++ b/src/main/java/build/buf/protovalidate/FieldEvaluator.java @@ -26,7 +26,7 @@ import org.jspecify.annotations.Nullable; /** Performs validation on a single message field, defined by its descriptor. */ -class FieldEvaluator implements Evaluator { +final class FieldEvaluator implements Evaluator { private static final FieldDescriptor REQUIRED_DESCRIPTOR = FieldRules.getDescriptor().findFieldByNumber(FieldRules.REQUIRED_FIELD_NUMBER); @@ -38,7 +38,7 @@ class FieldEvaluator implements Evaluator { private final RuleViolationHelper helper; /** The {@link ValueEvaluator} to apply to the field's value */ - public final ValueEvaluator valueEvaluator; + final ValueEvaluator valueEvaluator; /** The {@link FieldDescriptor} targeted by this evaluator */ private final FieldDescriptor descriptor; diff --git a/src/main/java/build/buf/protovalidate/FieldPathUtils.java b/src/main/java/build/buf/protovalidate/FieldPathUtils.java index 4706cc5c..27e321c4 100644 --- a/src/main/java/build/buf/protovalidate/FieldPathUtils.java +++ b/src/main/java/build/buf/protovalidate/FieldPathUtils.java @@ -78,7 +78,7 @@ static String fieldPathString(FieldPath fieldPath) { * @param fieldDescriptor The field descriptor to generate a field path element for. * @return The field path element that corresponds to the provided field descriptor. */ - public static FieldPathElement fieldPathElement(Descriptors.FieldDescriptor fieldDescriptor) { + static FieldPathElement fieldPathElement(Descriptors.FieldDescriptor fieldDescriptor) { String name; if (fieldDescriptor.isExtension()) { name = "[" + fieldDescriptor.getFullName() + "]"; @@ -100,7 +100,7 @@ public static FieldPathElement fieldPathElement(Descriptors.FieldDescriptor fiel * @param rulePathElements Rule path elements to prepend. * @return For convenience, the list of violations passed into the violations parameter. */ - public static List updatePaths( + static List updatePaths( List violations, @Nullable FieldPathElement fieldPathElement, List rulePathElements) { diff --git a/src/main/java/build/buf/protovalidate/Format.java b/src/main/java/build/buf/protovalidate/Format.java index f3dd494f..a7e9be24 100644 --- a/src/main/java/build/buf/protovalidate/Format.java +++ b/src/main/java/build/buf/protovalidate/Format.java @@ -319,7 +319,7 @@ private static String formatExponential(Object val, int precision) throws CelEva return str.get(); } String pattern = "%." + precision + "e"; - return String.format(pattern, (Double) val); + return String.format(pattern, val); } else { throw new CelEvaluationException( "error during formatting: scientific clause can only be used on doubles, was given " diff --git a/src/main/java/build/buf/protovalidate/ListEvaluator.java b/src/main/java/build/buf/protovalidate/ListEvaluator.java index 52bc3e21..0a08c050 100644 --- a/src/main/java/build/buf/protovalidate/ListEvaluator.java +++ b/src/main/java/build/buf/protovalidate/ListEvaluator.java @@ -24,7 +24,7 @@ import java.util.Objects; /** Performs validation on the elements of a repeated field. */ -class ListEvaluator implements Evaluator { +final class ListEvaluator implements Evaluator { /** Rule path to repeated rules */ private static final FieldPath REPEATED_ITEMS_RULE_PATH = FieldPath.newBuilder() diff --git a/src/main/java/build/buf/protovalidate/MapEvaluator.java b/src/main/java/build/buf/protovalidate/MapEvaluator.java index 8bc7ea66..425a67a5 100644 --- a/src/main/java/build/buf/protovalidate/MapEvaluator.java +++ b/src/main/java/build/buf/protovalidate/MapEvaluator.java @@ -28,7 +28,7 @@ import java.util.stream.Collectors; /** Performs validation on a map field's key-value pairs. */ -class MapEvaluator implements Evaluator { +final class MapEvaluator implements Evaluator { /** Rule path to map key rules */ private static final FieldPath MAP_KEYS_RULE_PATH = FieldPath.newBuilder() @@ -87,7 +87,7 @@ class MapEvaluator implements Evaluator { * * @return The key evaluator. */ - public ValueEvaluator getKeyEvaluator() { + ValueEvaluator getKeyEvaluator() { return keyEvaluator; } @@ -96,7 +96,7 @@ public ValueEvaluator getKeyEvaluator() { * * @return The value evaluator. */ - public ValueEvaluator getValueEvaluator() { + ValueEvaluator getValueEvaluator() { return valueEvaluator; } diff --git a/src/main/java/build/buf/protovalidate/MessageEvaluator.java b/src/main/java/build/buf/protovalidate/MessageEvaluator.java index fc496c2d..42fb0b7c 100644 --- a/src/main/java/build/buf/protovalidate/MessageEvaluator.java +++ b/src/main/java/build/buf/protovalidate/MessageEvaluator.java @@ -19,7 +19,7 @@ import java.util.List; /** Performs validation on a {@link com.google.protobuf.Message}. */ -class MessageEvaluator implements Evaluator { +final class MessageEvaluator implements Evaluator { /** List of {@link Evaluator}s that are applied to a message. */ private final List evaluators = new ArrayList<>(); @@ -55,7 +55,7 @@ public List evaluate(Value val, boolean failFast) * * @param eval The evaluator to append. */ - public void append(Evaluator eval) { + void append(Evaluator eval) { evaluators.add(eval); } } diff --git a/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java b/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java index 23c9325b..61a079be 100644 --- a/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/MessageOneofEvaluator.java @@ -25,12 +25,12 @@ * A specialized {@link Evaluator} for applying {@code buf.validate.MessageOneofRule} to a {@link * com.google.protobuf.Message}. */ -class MessageOneofEvaluator implements Evaluator { +final class MessageOneofEvaluator implements Evaluator { /** List of fields that are part of the oneof */ - public final List fields; + final List fields; /** If at least one must be set. */ - public final boolean required; + final boolean required; MessageOneofEvaluator(List fields, boolean required) { this.fields = fields; diff --git a/src/main/java/build/buf/protovalidate/MessageValue.java b/src/main/java/build/buf/protovalidate/MessageValue.java index de57f26d..9f4f924a 100644 --- a/src/main/java/build/buf/protovalidate/MessageValue.java +++ b/src/main/java/build/buf/protovalidate/MessageValue.java @@ -32,7 +32,7 @@ final class MessageValue implements Value { * * @param value The message value. */ - public MessageValue(Message value) { + MessageValue(Message value) { this.value = value; } diff --git a/src/main/java/build/buf/protovalidate/NowVariable.java b/src/main/java/build/buf/protovalidate/NowVariable.java index 8330d0c5..30ed02b8 100644 --- a/src/main/java/build/buf/protovalidate/NowVariable.java +++ b/src/main/java/build/buf/protovalidate/NowVariable.java @@ -24,15 +24,15 @@ * {@link NowVariable} implements {@link CelVariableResolver}, providing a lazily produced timestamp * for accessing the variable `now` that's constant within an evaluation. */ -class NowVariable implements CelVariableResolver { +final class NowVariable implements CelVariableResolver { /** The name of the 'now' variable. */ - public static final String NOW_NAME = "now"; + static final String NOW_NAME = "now"; /** The resolved value of the 'now' variable. */ @Nullable private Timestamp now; /** Creates an instance of a "now" variable. */ - public NowVariable() {} + NowVariable() {} @Override public Optional find(String name) { diff --git a/src/main/java/build/buf/protovalidate/ObjectValue.java b/src/main/java/build/buf/protovalidate/ObjectValue.java index 07829d34..6b41042f 100644 --- a/src/main/java/build/buf/protovalidate/ObjectValue.java +++ b/src/main/java/build/buf/protovalidate/ObjectValue.java @@ -41,7 +41,7 @@ final class ObjectValue implements Value { * @param fieldDescriptor The field descriptor for the value. * @param value The value associated with the field descriptor. */ - public ObjectValue(Descriptors.FieldDescriptor fieldDescriptor, Object value) { + ObjectValue(Descriptors.FieldDescriptor fieldDescriptor, Object value) { this.fieldDescriptor = fieldDescriptor; this.value = value; } diff --git a/src/main/java/build/buf/protovalidate/OneofEvaluator.java b/src/main/java/build/buf/protovalidate/OneofEvaluator.java index 2b6a10c9..3041e954 100644 --- a/src/main/java/build/buf/protovalidate/OneofEvaluator.java +++ b/src/main/java/build/buf/protovalidate/OneofEvaluator.java @@ -22,7 +22,7 @@ import java.util.List; /** {@link OneofEvaluator} performs validation on a oneof union. */ -class OneofEvaluator implements Evaluator { +final class OneofEvaluator implements Evaluator { /** The {@link OneofDescriptor} targeted by this evaluator. */ private final OneofDescriptor descriptor; @@ -35,7 +35,7 @@ class OneofEvaluator implements Evaluator { * @param descriptor The targeted oneof descriptor. * @param required Indicates whether a member of the oneof must be set. */ - public OneofEvaluator(OneofDescriptor descriptor, boolean required) { + OneofEvaluator(OneofDescriptor descriptor, boolean required) { this.descriptor = descriptor; this.required = required; } diff --git a/src/main/java/build/buf/protovalidate/ProtoAdapter.java b/src/main/java/build/buf/protovalidate/ProtoAdapter.java index 0c20ce79..d45d9804 100644 --- a/src/main/java/build/buf/protovalidate/ProtoAdapter.java +++ b/src/main/java/build/buf/protovalidate/ProtoAdapter.java @@ -30,7 +30,7 @@ */ final class ProtoAdapter { /** Converts a protobuf field value to CEL compatible value. */ - public static Object toCel(Descriptors.FieldDescriptor fieldDescriptor, Object value) { + static Object toCel(Descriptors.FieldDescriptor fieldDescriptor, Object value) { Descriptors.FieldDescriptor.Type type = fieldDescriptor.getType(); if (fieldDescriptor.isMapField()) { List input = @@ -64,7 +64,7 @@ public static Object toCel(Descriptors.FieldDescriptor fieldDescriptor, Object v } /** Converts a scalar type to cel value. */ - public static Object scalarToCel(Descriptors.FieldDescriptor.Type type, Object value) { + static Object scalarToCel(Descriptors.FieldDescriptor.Type type, Object value) { switch (type) { case ENUM: if (value instanceof Descriptors.EnumValueDescriptor) { diff --git a/src/main/java/build/buf/protovalidate/RuleCache.java b/src/main/java/build/buf/protovalidate/RuleCache.java index 9618b473..f3c2c726 100644 --- a/src/main/java/build/buf/protovalidate/RuleCache.java +++ b/src/main/java/build/buf/protovalidate/RuleCache.java @@ -38,13 +38,13 @@ import org.jspecify.annotations.Nullable; /** A build-through cache for computed standard rules. */ -class RuleCache { +final class RuleCache { private static class CelRule { - public final AstExpression astExpression; - public final FieldDescriptor field; - public final FieldPath rulePath; + final AstExpression astExpression; + final FieldDescriptor field; + final FieldPath rulePath; - public CelRule(AstExpression astExpression, FieldDescriptor field, FieldPath rulePath) { + private CelRule(AstExpression astExpression, FieldDescriptor field, FieldPath rulePath) { this.astExpression = astExpression; this.field = field; this.rulePath = rulePath; @@ -83,7 +83,7 @@ public CelRule(AstExpression astExpression, FieldDescriptor field, FieldPath rul * @param cel The CEL environment for evaluation. * @param config The configuration to use for the rule cache. */ - public RuleCache(Cel cel, Config config) { + RuleCache(Cel cel, Config config) { this.cel = cel; this.typeRegistry = config.getTypeRegistry(); this.extensionRegistry = config.getExtensionRegistry(); @@ -100,7 +100,7 @@ public RuleCache(Cel cel, Config config) { * @return The list of compiled programs. * @throws CompilationException If the rules fail to compile. */ - public List compile( + List compile( FieldDescriptor fieldDescriptor, FieldRules fieldRules, boolean forItems) throws CompilationException { ResolvedRule resolved = resolveRules(fieldDescriptor, fieldRules, forItems); diff --git a/src/main/java/build/buf/protovalidate/RuleResolver.java b/src/main/java/build/buf/protovalidate/RuleResolver.java index 17091fc6..e33c2256 100644 --- a/src/main/java/build/buf/protovalidate/RuleResolver.java +++ b/src/main/java/build/buf/protovalidate/RuleResolver.java @@ -28,7 +28,7 @@ import com.google.protobuf.MessageLite; /** Manages the resolution of protovalidate rules. */ -class RuleResolver { +final class RuleResolver { private static final ExtensionRegistry EXTENSION_REGISTRY = ExtensionRegistry.newInstance(); static { diff --git a/src/main/java/build/buf/protovalidate/RuleViolation.java b/src/main/java/build/buf/protovalidate/RuleViolation.java index 67be135b..2f7e5de2 100644 --- a/src/main/java/build/buf/protovalidate/RuleViolation.java +++ b/src/main/java/build/buf/protovalidate/RuleViolation.java @@ -18,23 +18,22 @@ import build.buf.validate.FieldPathElement; import com.google.protobuf.Descriptors; import java.util.ArrayDeque; -import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Deque; import java.util.List; import java.util.Objects; import org.jspecify.annotations.Nullable; /** - * {@link RuleViolation} contains all of the collected information about an individual rule - * violation. + * {@link RuleViolation} contains all the collected information about an individual rule violation. */ -class RuleViolation implements Violation { +final class RuleViolation implements Violation { /** Static value to return when there are no violations. */ - public static final List NO_VIOLATIONS = new ArrayList<>(); + static final List NO_VIOLATIONS = Collections.emptyList(); /** {@link FieldValue} represents a Protobuf field value inside a Protobuf message. */ - public static class FieldValue implements Violation.FieldValue { + static class FieldValue implements Violation.FieldValue { private final @Nullable Object value; private final Descriptors.FieldDescriptor descriptor; @@ -44,7 +43,7 @@ public static class FieldValue implements Violation.FieldValue { * @param value Bare Protobuf field value of field. * @param descriptor Field descriptor pertaining to this field. */ - public FieldValue(@Nullable Object value, Descriptors.FieldDescriptor descriptor) { + FieldValue(@Nullable Object value, Descriptors.FieldDescriptor descriptor) { this.value = value; this.descriptor = descriptor; } @@ -55,7 +54,7 @@ public FieldValue(@Nullable Object value, Descriptors.FieldDescriptor descriptor * * @param value A {@link Value} to create this {@link FieldValue} from. */ - public FieldValue(Value value) { + FieldValue(Value value) { this.value = value.value(Object.class); this.descriptor = Objects.requireNonNull(value.fieldDescriptor()); } @@ -76,7 +75,7 @@ public Descriptors.FieldDescriptor getDescriptor() { private final @Nullable FieldValue ruleValue; /** Builds a Violation instance. */ - public static class Builder { + static class Builder { private @Nullable String ruleId; private @Nullable String message; private boolean forKey = false; @@ -91,7 +90,7 @@ public static class Builder { * @param ruleId Rule ID value to use. * @return The builder. */ - public Builder setRuleId(String ruleId) { + Builder setRuleId(String ruleId) { this.ruleId = ruleId; return this; } @@ -102,7 +101,7 @@ public Builder setRuleId(String ruleId) { * @param message Message value to use. * @return The builder. */ - public Builder setMessage(String message) { + Builder setMessage(String message) { this.message = message; return this; } @@ -113,7 +112,7 @@ public Builder setMessage(String message) { * @param forKey If true, signals that the resulting violation is for a map key. * @return The builder. */ - public Builder setForKey(boolean forKey) { + Builder setForKey(boolean forKey) { this.forKey = forKey; return this; } @@ -124,8 +123,7 @@ public Builder setForKey(boolean forKey) { * @param fieldPathElements Field path elements to add. * @return The builder. */ - public Builder addAllFieldPathElements( - Collection fieldPathElements) { + Builder addAllFieldPathElements(Collection fieldPathElements) { this.fieldPath.addAll(fieldPathElements); return this; } @@ -136,7 +134,7 @@ public Builder addAllFieldPathElements( * @param fieldPathElement A field path element to add to the beginning of the field path. * @return The builder. */ - public Builder addFirstFieldPathElement(@Nullable FieldPathElement fieldPathElement) { + Builder addFirstFieldPathElement(@Nullable FieldPathElement fieldPathElement) { if (fieldPathElement != null) { fieldPath.addFirst(fieldPathElement); } @@ -149,7 +147,7 @@ public Builder addFirstFieldPathElement(@Nullable FieldPathElement fieldPathElem * @param rulePathElements Field path elements to add. * @return The builder. */ - public Builder addAllRulePathElements(Collection rulePathElements) { + Builder addAllRulePathElements(Collection rulePathElements) { rulePath.addAll(rulePathElements); return this; } @@ -160,7 +158,7 @@ public Builder addAllRulePathElements(Collection rul * @param rulePathElements A field path element to add to the beginning of the rule path. * @return The builder. */ - public Builder addFirstRulePathElement(FieldPathElement rulePathElements) { + Builder addFirstRulePathElement(FieldPathElement rulePathElements) { rulePath.addFirst(rulePathElements); return this; } @@ -171,7 +169,7 @@ public Builder addFirstRulePathElement(FieldPathElement rulePathElements) { * @param fieldValue The field value corresponding to this violation. * @return The builder. */ - public Builder setFieldValue(@Nullable FieldValue fieldValue) { + Builder setFieldValue(@Nullable FieldValue fieldValue) { this.fieldValue = fieldValue; return this; } @@ -182,7 +180,7 @@ public Builder setFieldValue(@Nullable FieldValue fieldValue) { * @param ruleValue The rule value corresponding to this violation. * @return The builder. */ - public Builder setRuleValue(@Nullable FieldValue ruleValue) { + Builder setRuleValue(@Nullable FieldValue ruleValue) { this.ruleValue = ruleValue; return this; } @@ -192,7 +190,7 @@ public Builder setRuleValue(@Nullable FieldValue ruleValue) { * * @return A Violation instance. */ - public RuleViolation build() { + RuleViolation build() { build.buf.validate.Violation.Builder protoBuilder = build.buf.validate.Violation.newBuilder(); if (ruleId != null) { protoBuilder.setRuleId(ruleId); @@ -220,7 +218,7 @@ private Builder() {} * * @return A new, empty {@link Builder}. */ - public static Builder newBuilder() { + static Builder newBuilder() { return new Builder(); } diff --git a/src/main/java/build/buf/protovalidate/RuleViolationHelper.java b/src/main/java/build/buf/protovalidate/RuleViolationHelper.java index 6425fda9..3e2591a1 100644 --- a/src/main/java/build/buf/protovalidate/RuleViolationHelper.java +++ b/src/main/java/build/buf/protovalidate/RuleViolationHelper.java @@ -20,7 +20,7 @@ import java.util.List; import org.jspecify.annotations.Nullable; -class RuleViolationHelper { +final class RuleViolationHelper { private static final List EMPTY_PREFIX = new ArrayList<>(); private final @Nullable FieldPath rulePrefix; @@ -41,11 +41,6 @@ class RuleViolationHelper { } } - RuleViolationHelper(@Nullable FieldPath rulePrefix) { - this.rulePrefix = rulePrefix; - this.fieldPathElement = null; - } - @Nullable FieldPathElement getFieldPathElement() { return fieldPathElement; } diff --git a/src/main/java/build/buf/protovalidate/UnknownDescriptorEvaluator.java b/src/main/java/build/buf/protovalidate/UnknownDescriptorEvaluator.java index b8652d93..d3ea3f08 100644 --- a/src/main/java/build/buf/protovalidate/UnknownDescriptorEvaluator.java +++ b/src/main/java/build/buf/protovalidate/UnknownDescriptorEvaluator.java @@ -23,7 +23,7 @@ * An {@link Evaluator} for an unknown descriptor. This is returned only if lazy-building of * evaluators has been disabled and an unknown descriptor is encountered. */ -class UnknownDescriptorEvaluator implements Evaluator { +final class UnknownDescriptorEvaluator implements Evaluator { /** The descriptor targeted by this evaluator. */ private final Descriptor desc; diff --git a/src/main/java/build/buf/protovalidate/Uri.java b/src/main/java/build/buf/protovalidate/Uri.java index 60a127e2..91a618d5 100644 --- a/src/main/java/build/buf/protovalidate/Uri.java +++ b/src/main/java/build/buf/protovalidate/Uri.java @@ -22,7 +22,7 @@ /** Ipv6 is a class used to parse a given string to determine if it is a URI or URI reference. */ final class Uri { - private String str; + private final String str; private int index; private boolean pctEncodedFound; @@ -343,9 +343,7 @@ private boolean host() { // RFC 3986: // > URI producing applications must not use percent-encoding in host // > unless it is used to represent a UTF-8 character sequence. - if (!this.checkHostPctEncoded(rawHost)) { - return false; - } + return this.checkHostPctEncoded(rawHost); } return true; diff --git a/src/main/java/build/buf/protovalidate/ValidateLibrary.java b/src/main/java/build/buf/protovalidate/ValidateLibrary.java index 0e7467a3..3bf9b015 100644 --- a/src/main/java/build/buf/protovalidate/ValidateLibrary.java +++ b/src/main/java/build/buf/protovalidate/ValidateLibrary.java @@ -30,10 +30,10 @@ * Custom {@link CelCompilerLibrary} and {@link CelRuntimeLibrary}. Provides all the custom * extension function definitions and overloads. */ -class ValidateLibrary implements CelCompilerLibrary, CelRuntimeLibrary { +final class ValidateLibrary implements CelCompilerLibrary, CelRuntimeLibrary { /** Creates a ValidateLibrary with all custom declarations and overloads. */ - public ValidateLibrary() {} + ValidateLibrary() {} @Override public void setParserOptions(CelParserBuilder parserBuilder) { diff --git a/src/main/java/build/buf/protovalidate/ValidatorImpl.java b/src/main/java/build/buf/protovalidate/ValidatorImpl.java index 6ed84ad1..9be4b04b 100644 --- a/src/main/java/build/buf/protovalidate/ValidatorImpl.java +++ b/src/main/java/build/buf/protovalidate/ValidatorImpl.java @@ -23,7 +23,7 @@ import java.util.ArrayList; import java.util.List; -class ValidatorImpl implements Validator { +final class ValidatorImpl implements Validator { /** evaluatorBuilder is the builder used to construct the evaluator for a given message. */ private final EvaluatorBuilder evaluatorBuilder; @@ -56,7 +56,6 @@ class ValidatorImpl implements Validator { this.failFast = config.isFailFast(); } - /** {@inheritDoc} */ @Override public ValidationResult validate(Message msg) throws ValidationException { if (msg == null) { diff --git a/src/main/java/build/buf/protovalidate/ValueEvaluator.java b/src/main/java/build/buf/protovalidate/ValueEvaluator.java index a967073e..4f69cc88 100644 --- a/src/main/java/build/buf/protovalidate/ValueEvaluator.java +++ b/src/main/java/build/buf/protovalidate/ValueEvaluator.java @@ -26,7 +26,7 @@ * {@link ValueEvaluator} performs validation on any concrete value contained within a singular * field, repeated elements, or the keys/values of a map. */ -class ValueEvaluator implements Evaluator { +final class ValueEvaluator implements Evaluator { /** The {@link Descriptors.FieldDescriptor} targeted by this evaluator */ private final Descriptors.@Nullable FieldDescriptor descriptor; @@ -51,15 +51,15 @@ class ValueEvaluator implements Evaluator { this.nestedRule = nestedRule; } - public Descriptors.@Nullable FieldDescriptor getDescriptor() { + Descriptors.@Nullable FieldDescriptor getDescriptor() { return descriptor; } - public @Nullable FieldPath getNestedRule() { + @Nullable FieldPath getNestedRule() { return nestedRule; } - public boolean hasNestedRule() { + boolean hasNestedRule() { return this.nestedRule != null; } @@ -93,13 +93,13 @@ public List evaluate(Value val, boolean failFast) * * @param eval The evaluator to append. */ - public void append(Evaluator eval) { + void append(Evaluator eval) { if (!eval.tautology()) { this.evaluators.add(eval); } } - public void setIgnoreEmpty(Object zero) { + void setIgnoreEmpty(Object zero) { this.ignoreEmpty = true; this.zero = zero; } diff --git a/src/main/java/build/buf/protovalidate/Variable.java b/src/main/java/build/buf/protovalidate/Variable.java index 8e71a46a..c231139f 100644 --- a/src/main/java/build/buf/protovalidate/Variable.java +++ b/src/main/java/build/buf/protovalidate/Variable.java @@ -22,18 +22,15 @@ * {@link Variable} implements {@link CelVariableResolver}, providing a lightweight named variable * to cel.Program executions. */ -class Variable implements CelVariableResolver { +final class Variable implements CelVariableResolver { /** The {@value} variable in CEL. */ - public static final String THIS_NAME = "this"; + static final String THIS_NAME = "this"; /** The {@value} variable in CEL. */ - public static final String RULES_NAME = "rules"; + static final String RULES_NAME = "rules"; /** The {@value} variable in CEL. */ - public static final String RULE_NAME = "rule"; - - /** The {@value} variable in CEL. */ - public static final String NOW_NAME = "now"; + static final String RULE_NAME = "rule"; /** The variable's name */ private final String name; @@ -53,7 +50,7 @@ private Variable(String name, @Nullable Object val) { * @param val the value. * @return {@link Variable}. */ - public static CelVariableResolver newThisVariable(@Nullable Object val) { + static CelVariableResolver newThisVariable(@Nullable Object val) { return CelVariableResolver.hierarchicalVariableResolver( new NowVariable(), new Variable(THIS_NAME, val)); } @@ -64,7 +61,7 @@ public static CelVariableResolver newThisVariable(@Nullable Object val) { * @param val the value. * @return {@link Variable}. */ - public static CelVariableResolver newRulesVariable(Object val) { + static CelVariableResolver newRulesVariable(Object val) { return new Variable(RULES_NAME, val); } @@ -75,7 +72,7 @@ public static CelVariableResolver newRulesVariable(Object val) { * @param val the value of the "rule" variable. * @return {@link Variable}. */ - public static CelVariableResolver newRuleVariable(Object rules, Object val) { + static CelVariableResolver newRuleVariable(Object rules, Object val) { return CelVariableResolver.hierarchicalVariableResolver( newRulesVariable(rules), new Variable(RULE_NAME, val)); } diff --git a/src/main/java/build/buf/protovalidate/Violation.java b/src/main/java/build/buf/protovalidate/Violation.java index d9666c52..cbc9d6d7 100644 --- a/src/main/java/build/buf/protovalidate/Violation.java +++ b/src/main/java/build/buf/protovalidate/Violation.java @@ -17,9 +17,7 @@ import com.google.protobuf.Descriptors; import org.jspecify.annotations.Nullable; -/** - * {@link Violation} provides all of the collected information about an individual rule violation. - */ +/** {@link Violation} provides all the collected information about an individual rule violation. */ public interface Violation { /** {@link FieldValue} represents a Protobuf field value inside a Protobuf message. */ interface FieldValue { From 5cf092c6989b0cb87f7958831b47e55bd5232b4a Mon Sep 17 00:00:00 2001 From: "Philip K. Warren" Date: Wed, 11 Jun 2025 17:33:30 -0500 Subject: [PATCH 2/3] Remove another mutable constant --- .../java/build/buf/protovalidate/RuleViolationHelper.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/RuleViolationHelper.java b/src/main/java/build/buf/protovalidate/RuleViolationHelper.java index 3e2591a1..9a50a7d2 100644 --- a/src/main/java/build/buf/protovalidate/RuleViolationHelper.java +++ b/src/main/java/build/buf/protovalidate/RuleViolationHelper.java @@ -16,12 +16,12 @@ import build.buf.validate.FieldPath; import build.buf.validate.FieldPathElement; -import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.jspecify.annotations.Nullable; final class RuleViolationHelper { - private static final List EMPTY_PREFIX = new ArrayList<>(); + private static final List EMPTY_PREFIX = Collections.emptyList(); private final @Nullable FieldPath rulePrefix; From bfd06e8a52708cda1a78062daa794b0dad58b68a Mon Sep 17 00:00:00 2001 From: "Philip K. Warren" Date: Wed, 11 Jun 2025 17:39:51 -0500 Subject: [PATCH 3/3] Fix a few more warnings --- .../java/build/buf/protovalidate/CustomOverload.java | 12 +++--------- src/main/java/build/buf/protovalidate/Ipv4.java | 6 +++--- src/main/java/build/buf/protovalidate/Ipv6.java | 6 +++--- .../build/buf/protovalidate/ValidateLibrary.java | 2 +- 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/main/java/build/buf/protovalidate/CustomOverload.java b/src/main/java/build/buf/protovalidate/CustomOverload.java index 96ebc39c..2bf7f93a 100644 --- a/src/main/java/build/buf/protovalidate/CustomOverload.java +++ b/src/main/java/build/buf/protovalidate/CustomOverload.java @@ -203,9 +203,7 @@ private static CelFunctionBinding celContainsBytes() { "contains_bytes", ByteString.class, ByteString.class, - (receiver, param) -> { - return bytesContains(receiver.toByteArray(), param.toByteArray()); - }); + (receiver, param) -> bytesContains(receiver.toByteArray(), param.toByteArray())); } static boolean bytesContains(byte[] arr, byte[] subArr) { @@ -286,9 +284,7 @@ private static CelFunctionBinding celIsIpPrefixInt() { "is_ip_prefix_int", String.class, Long.class, - (prefix, version) -> { - return isIpPrefix(prefix, version, false); - }); + (prefix, version) -> isIpPrefix(prefix, version, false)); } /** @@ -301,9 +297,7 @@ private static CelFunctionBinding celIsIpPrefixBool() { "is_ip_prefix_bool", String.class, Boolean.class, - (prefix, strict) -> { - return isIpPrefix(prefix, 0L, strict); - }); + (prefix, strict) -> isIpPrefix(prefix, 0L, strict)); } /** diff --git a/src/main/java/build/buf/protovalidate/Ipv4.java b/src/main/java/build/buf/protovalidate/Ipv4.java index 538c38ae..747c3f8e 100644 --- a/src/main/java/build/buf/protovalidate/Ipv4.java +++ b/src/main/java/build/buf/protovalidate/Ipv4.java @@ -22,14 +22,14 @@ * prefix. */ final class Ipv4 { - private String str; + private final String str; private int index; - private List octets; + private final List octets; private int prefixLen; Ipv4(String str) { this.str = str; - this.octets = new ArrayList(); + this.octets = new ArrayList<>(); } /** diff --git a/src/main/java/build/buf/protovalidate/Ipv6.java b/src/main/java/build/buf/protovalidate/Ipv6.java index b019cc01..af46fd2a 100644 --- a/src/main/java/build/buf/protovalidate/Ipv6.java +++ b/src/main/java/build/buf/protovalidate/Ipv6.java @@ -23,10 +23,10 @@ * prefix. */ final class Ipv6 { - private String str; + private final String str; private int index; // 16-bit pieces found - private List pieces; + private final List pieces; // number of 16-bit pieces found when double colon was found private int doubleColonAt; private boolean doubleColonSeen; @@ -40,7 +40,7 @@ final class Ipv6 { Ipv6(String str) { this.str = str; - this.pieces = new ArrayList(); + this.pieces = new ArrayList<>(); this.doubleColonAt = -1; this.dottedRaw = ""; } diff --git a/src/main/java/build/buf/protovalidate/ValidateLibrary.java b/src/main/java/build/buf/protovalidate/ValidateLibrary.java index 3bf9b015..ec5e00d1 100644 --- a/src/main/java/build/buf/protovalidate/ValidateLibrary.java +++ b/src/main/java/build/buf/protovalidate/ValidateLibrary.java @@ -63,7 +63,7 @@ public void setRuntimeOptions(CelRuntimeBuilder runtimeBuilder) { .setStandardFunctions( CelStandardFunctions.newBuilder() .filterFunctions( - // CEL doesn't validate, that the bytes are valid utf-8, so we provide out own + // CEL doesn't validate, that the bytes are valid utf-8, so we provide our own // implementation. (function, overload) -> function != StandardFunction.STRING