Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,10 @@ mavenPublishing {

dependencies {
annotationProcessor(libs.nullaway)
api(libs.jspecify)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jspecify appears to be the newer long term replacement for javax annotations, so switched to it. Previously we pulled these annotations in through Guava's jsr305 dependency.

api(libs.protobuf.java)
implementation(enforcedPlatform(libs.cel))
implementation(libs.cel.core)
implementation(libs.guava)

buf("build.buf:buf:${libs.versions.buf.get()}:${osdetector.classifier}@exe")

Expand All @@ -269,5 +269,5 @@ dependencies {
testImplementation("org.junit.jupiter:junit-jupiter")
testRuntimeOnly("org.junit.platform:junit-platform-launcher")

errorprone(libs.errorprone)
errorprone(libs.errorprone.core)
}
10 changes: 8 additions & 2 deletions conformance/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ plugins {
alias(libs.plugins.osdetector)
}

// Conformance tests aren't bound by lowest common library version.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason the tests themselves couldn't target a later version of Java.

java {
sourceCompatibility = JavaVersion.VERSION_21
targetCompatibility = JavaVersion.VERSION_21
}

val buf: Configuration by configurations.creating

tasks.register("configureBuf") {
Expand Down Expand Up @@ -116,7 +122,7 @@ configure<SpotlessExtension> {

dependencies {
implementation(project(":"))
implementation(libs.guava)
implementation(libs.errorprone.annotations)
implementation(libs.protobuf.java)

implementation(libs.assertj)
Expand All @@ -127,5 +133,5 @@ dependencies {
testImplementation("org.junit.jupiter:junit-jupiter")
testRuntimeOnly("org.junit.platform:junit-platform-launcher")

errorprone(libs.errorprone)
errorprone(libs.errorprone.core)
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import build.buf.validate.conformance.harness.TestConformanceRequest;
import build.buf.validate.conformance.harness.TestConformanceResponse;
import build.buf.validate.conformance.harness.TestResult;
import com.google.common.base.Splitter;
import com.google.errorprone.annotations.FormatMethod;
import com.google.protobuf.Any;
import com.google.protobuf.ByteString;
Expand All @@ -34,7 +33,6 @@
import com.google.protobuf.InvalidProtocolBufferException;
import com.google.protobuf.TypeRegistry;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class Main {
Expand Down Expand Up @@ -84,8 +82,11 @@ static TestConformanceResponse testConformance(TestConformanceRequest request) {
static TestResult testCase(
Validator validator, Map<String, Descriptors.Descriptor> fileDescriptors, Any testCase)
throws InvalidProtocolBufferException {
List<String> urlParts = Splitter.on('/').limit(2).splitToList(testCase.getTypeUrl());
String fullName = urlParts.get(urlParts.size() - 1);
String fullName = testCase.getTypeUrl();
int slash = fullName.indexOf('/');
if (slash != -1) {
fullName = fullName.substring(slash + 1);
}
Descriptors.Descriptor descriptor = fileDescriptors.get(fullName);
if (descriptor == null) {
return unexpectedErrorResult("Unable to find descriptor: %s", fullName);
Expand Down
6 changes: 4 additions & 2 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
assertj = "3.27.3"
buf = "1.52.1"
cel = "0.5.1"
error-prone = "2.37.0"
junit = "5.12.1"
maven-publish = "0.31.0"
# When updating, make sure to update versions in the following files to match and regenerate code with 'make generate'.
Expand All @@ -16,8 +17,9 @@ assertj = { module = "org.assertj:assertj-core", version.ref = "assertj" }
buf = { module = "build.buf:buf", version.ref = "buf" }
cel = { module = "org.projectnessie.cel:cel-bom", version.ref = "cel" }
cel-core = { module = "org.projectnessie.cel:cel-core" }
errorprone = { module = "com.google.errorprone:error_prone_core", version = "2.37.0" }
guava = { module = "com.google.guava:guava", version = "33.4.0-jre" }
errorprone-annotations = { module = "com.google.errorprone:error_prone_annotations", version.ref = "error-prone" }
errorprone-core = { module = "com.google.errorprone:error_prone_core", version.ref = "error-prone" }
jspecify = { module ="org.jspecify:jspecify", version = "1.0.0" }
junit-bom = { module = "org.junit:junit-bom", version.ref = "junit" }
maven-plugin = { module = "com.vanniktech:gradle-maven-publish-plugin", version.ref = "maven-publish" }
nullaway = { module = "com.uber.nullaway:nullaway", version = "0.12.6" }
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/CelPrograms.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import build.buf.protovalidate.exceptions.ExecutionException;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/** Evaluator that executes a {@link CompiledProgram}. */
class CelPrograms implements Evaluator {
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/build/buf/protovalidate/CompiledProgram.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import build.buf.protovalidate.exceptions.ExecutionException;
import build.buf.validate.FieldPath;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.Program;
import org.projectnessie.cel.common.types.Err;
import org.projectnessie.cel.common.types.ref.Val;
Expand Down Expand Up @@ -63,8 +63,7 @@ public CompiledProgram(
* violations.
* @throws ExecutionException If the evaluation of the CEL program fails with an error.
*/
@Nullable
public ConstraintViolation.Builder eval(Value fieldValue, Variable bindings)
public ConstraintViolation.@Nullable Builder eval(Value fieldValue, Variable bindings)
throws ExecutionException {
Program.EvalResult evalResult = program.eval(bindings);
Val val = evalResult.getVal();
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/build/buf/protovalidate/ConstraintCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.Ast;
import org.projectnessie.cel.Env;
import org.projectnessie.cel.EnvOption;
Expand Down Expand Up @@ -199,7 +199,7 @@ public List<CompiledProgram> compile(
return celRules;
}

private @Nullable build.buf.validate.PredefinedConstraints getFieldConstraints(
private build.buf.validate.@Nullable PredefinedConstraints getFieldConstraints(
FieldDescriptor constraintFieldDesc) throws CompilationException {
DescriptorProtos.FieldOptions options = constraintFieldDesc.getOptions();
// If the protovalidate field option is unknown, reparse options using our extension registry.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import java.util.Deque;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* {@link ConstraintViolation} contains all of the collected information about an individual
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import build.buf.validate.FieldPathElement;
import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

class ConstraintViolationHelper {
private static final List<FieldPathElement> EMPTY_PREFIX = new ArrayList<>();
Expand Down Expand Up @@ -46,8 +46,7 @@ class ConstraintViolationHelper {
this.fieldPathElement = null;
}

@Nullable
FieldPathElement getFieldPathElement() {
@Nullable FieldPathElement getFieldPathElement() {
return fieldPathElement;
}

Expand Down
25 changes: 23 additions & 2 deletions src/main/java/build/buf/protovalidate/CustomOverload.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package build.buf.protovalidate;

import com.google.common.primitives.Bytes;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -205,12 +204,34 @@ private static Overload celContains() {
if (lhsType == TypeEnum.Bytes) {
byte[] receiver = (byte[]) lhs.value();
byte[] param = (byte[]) rhs.value();
return Types.boolOf(Bytes.indexOf(receiver, param) != -1);
return Types.boolOf(bytesContains(receiver, param));
}
return Err.noSuchOverload(lhs, OVERLOAD_CONTAINS, rhs);
});
}

static boolean bytesContains(byte[] arr, byte[] subArr) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main thing we got from Guava. Immutable types are great but for our usage we didn't require them anywhere. We could always (when upgrading to Java 11) switch to similar types on the List/Map methods.

if (subArr.length == 0) {
return true;
}
if (subArr.length > arr.length) {
return false;
}
for (int i = 0; i < arr.length - subArr.length + 1; i++) {
boolean found = true;
for (int j = 0; j < subArr.length; j++) {
if (arr[i + j] != subArr[j]) {
found = false;
break;
}
}
if (found) {
return true;
}
}
return false;
}

/**
* Creates a custom binary function overload for the "isHostname" operation.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import com.google.protobuf.Descriptors.OneofDescriptor;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.checker.Decls;

/**
Expand Down
20 changes: 8 additions & 12 deletions src/main/java/build/buf/protovalidate/EvaluatorBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import build.buf.validate.Ignore;
import build.buf.validate.MessageConstraints;
import build.buf.validate.OneofConstraints;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.ByteString;
import com.google.protobuf.Descriptors;
import com.google.protobuf.Descriptors.Descriptor;
Expand All @@ -35,8 +34,9 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.Env;
import org.projectnessie.cel.EnvOption;
import org.projectnessie.cel.checker.Decls;
Expand All @@ -47,7 +47,7 @@ class EvaluatorBuilder {
FieldPathUtils.fieldPathElement(
FieldConstraints.getDescriptor().findFieldByNumber(FieldConstraints.CEL_FIELD_NUMBER));

private volatile ImmutableMap<Descriptor, MessageEvaluator> evaluatorCache = ImmutableMap.of();
private volatile Map<Descriptor, MessageEvaluator> evaluatorCache = Collections.emptyMap();

private final Env env;
private final boolean disableLazy;
Expand Down Expand Up @@ -98,7 +98,7 @@ private Evaluator build(Descriptor desc) throws CompilationException {
return eval;
}
// Rebuild cache with this descriptor (and any of its dependencies).
ImmutableMap<Descriptor, MessageEvaluator> updatedCache =
Map<Descriptor, MessageEvaluator> updatedCache =
new DescriptorCacheBuilder(env, constraints, evaluatorCache).build(desc);
evaluatorCache = updatedCache;
eval = updatedCache.get(desc);
Expand All @@ -117,9 +117,7 @@ private static class DescriptorCacheBuilder {
private final HashMap<Descriptor, MessageEvaluator> cache;

private DescriptorCacheBuilder(
Env env,
ConstraintCache constraintCache,
ImmutableMap<Descriptor, MessageEvaluator> previousCache) {
Env env, ConstraintCache constraintCache, Map<Descriptor, MessageEvaluator> previousCache) {
this.env = Objects.requireNonNull(env, "env");
this.constraintCache = Objects.requireNonNull(constraintCache, "constraintCache");
this.cache = new HashMap<>(previousCache);
Expand All @@ -130,13 +128,13 @@ private DescriptorCacheBuilder(
* references).
*
* @param descriptor Descriptor used to build the cache.
* @return Immutable map of descriptors to evaluators.
* @return Unmodifiable map of descriptors to evaluators.
* @throws CompilationException If an error occurs compiling a constraint on the cache.
*/
public ImmutableMap<Descriptor, MessageEvaluator> build(Descriptor descriptor)
public Map<Descriptor, MessageEvaluator> build(Descriptor descriptor)
throws CompilationException {
createMessageEvaluator(descriptor);
return ImmutableMap.copyOf(cache);
return Collections.unmodifiableMap(cache);
}

private MessageEvaluator createMessageEvaluator(Descriptor desc) throws CompilationException {
Expand Down Expand Up @@ -234,12 +232,10 @@ private FieldEvaluator buildField(
return fieldEvaluator;
}

@SuppressWarnings("deprecation")
private boolean shouldSkip(FieldConstraints constraints) {
return constraints.getIgnore() == Ignore.IGNORE_ALWAYS;
}

@SuppressWarnings("deprecation")
private static boolean shouldIgnoreEmpty(FieldConstraints constraints) {
return constraints.getIgnore() == Ignore.IGNORE_IF_UNPOPULATED
|| constraints.getIgnore() == Ignore.IGNORE_IF_DEFAULT_VALUE;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/FieldEvaluator.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/** Performs validation on a single message field, defined by its descriptor. */
class FieldEvaluator implements Evaluator {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/FieldPathUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import build.buf.validate.FieldPathElement;
import com.google.protobuf.Descriptors;
import java.util.List;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/** Utility class for manipulating error paths in violations. */
final class FieldPathUtils {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/Ipv6.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import java.util.ArrayList;
import java.util.List;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* Ipv6 is a class used to parse a given string to determine if it is an IPv6 address or address
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/build/buf/protovalidate/MessageValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/** The {@link Value} type that contains a {@link com.google.protobuf.Message}. */
final class MessageValue implements Value {
Expand All @@ -37,7 +37,7 @@ public MessageValue(Message value) {
}

@Override
public @Nullable Descriptors.FieldDescriptor fieldDescriptor() {
public Descriptors.@Nullable FieldDescriptor fieldDescriptor() {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/NowVariable.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
package build.buf.protovalidate;

import java.time.Instant;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.common.types.TimestampT;
import org.projectnessie.cel.interpreter.Activation;
import org.projectnessie.cel.interpreter.ResolvedValue;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/build/buf/protovalidate/ObjectValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;
import org.projectnessie.cel.common.ULong;

/** The {@link Value} type that contains a field descriptor and its value. */
Expand Down
8 changes: 3 additions & 5 deletions src/main/java/build/buf/protovalidate/Value.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import com.google.protobuf.Message;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.jspecify.annotations.Nullable;

/**
* {@link Value} is a wrapper around a protobuf value that provides helper methods for accessing the
Expand All @@ -31,17 +31,15 @@ interface Value {
* @return The underlying {@link Descriptors.FieldDescriptor}. null if the underlying value is not
* a message field.
*/
@Nullable
Descriptors.FieldDescriptor fieldDescriptor();
Descriptors.@Nullable FieldDescriptor fieldDescriptor();

/**
* Get the underlying value as a {@link Message} type.
*
* @return The underlying {@link Message} value. null if the underlying value is not a {@link
* Message} type.
*/
@Nullable
Message messageValue();
@Nullable Message messageValue();

/**
* Get the underlying value and cast it to the class type.
Expand Down
Loading