diff --git a/client/dynamic-client-errorprone/README.md b/client/dynamic-client-errorprone/README.md new file mode 100644 index 000000000..a5222b37f --- /dev/null +++ b/client/dynamic-client-errorprone/README.md @@ -0,0 +1,132 @@ +# Dynamic client usage check (Error Prone) + +An **[Error Prone](https://errorprone.info) check** that validates `DynamicClient` usage against the Smithy model each +client is *actually built from*, at compile time — **without** any code generation. The dynamic client stays fully +runtime-driven and document-based; this check only reads the source being compiled and catches the mistakes that are +statically catchable. + +## What it checks + +Given code like: + +```java +DynamicClient client = DynamicClient.builder() + .serviceId(ShapeId.from("smithy.example#Sprockets")) + .model(Model.assembler().addUnparsedModel("demo.smithy", MODEL).assemble().unwrap()) + .build(); + +client.call("GetSprokcet", Map.of("id", "1")); // <-- compile error: typo, no such operation +client.call("GetSprocket", Map.of("idd", "1")); // <-- compile error: 'idd' not an input member +``` + +the check emits, anchored to the exact argument: + +``` +error: [DynamicClientUsage] Operation 'GetSprokcet' not found in service 'smithy.example#Sprockets'. + Known operations: [CreateSprocket, GetSprocket] + Did you mean 'GetSprocket'? <-- suggested auto-fix +error: [DynamicClientUsage] 'idd' is not a member of input 'GetSprocketInput' for operation 'GetSprocket'. + Known members: [id] +``` + +Concretely, per `call(...)` site whose receiver is a `DynamicClient` it can resolve: + +1. **operation name** — the first argument, when a String literal or `static final String` constant, must be an + operation on the resolved service (with a Levenshtein-based *"did you mean"* suggested fix); +2. **input keys** — when the second argument is a `Map.of("k", v, ...)` literal, every key must be a member of the + operation's input structure. + +## Why Error Prone (vs. a raw javac plugin) + +This started as a raw `com.sun.source.util.Plugin`. Porting to Error Prone kept the same idea but replaced hand-rolled +machinery with framework features: + +| Hand-rolled in the raw plugin | Provided by Error Prone | +|---|---| +| Custom string-constant folding | `ASTHelpers.constValue(...)` | +| Report-only diagnostics | **`SuggestedFix`** — an applyable "did you mean `GetSprocket`?" auto-fix | +| A `run.sh` shell driver | `CompilationTestHelper` with inline `// BUG: Diagnostic contains:` assertions | +| No suppression | `@SuppressWarnings("DynamicClientUsage")` for free | +| Manual `TaskListener` wiring | `@AutoService(BugChecker.class)` registration | + +The **model-resolution core** (`ModelResolver`, `ResolvedClient`, `MapLiteral`) is unchanged from the raw-plugin +prototype and has no dependency on Error Prone or javac internals — it takes a `constantResolver` function, so the same +core could back an Error Prone check, a javac plugin, or a standalone parser task. Error Prone is just the driver. + +## How it works + +- **Match** (`DynamicClientUsageChecker`) — a `MethodInvocationTreeMatcher` matching `call(...)` on `DynamicClient`. +- **Resolve the client** — walk from the `call` receiver to the local's `DynamicClient.builder()...build()` + initializer, read the `.model(...)` and `.serviceId(...)` arguments (handling `ShapeId.from("...")`). +- **Resolve + assemble the model** (`ModelResolver`) — statically read the `Model.assembler()...` chain + (`addUnparsedModel` / `addImport` / `discoverModels`, folding constants) and assemble it with the **real + `smithy-model` `ModelAssembler`** — the same code the runtime uses, so no drift. Cached per client. +- **Validate** — check the operation name and any `Map.of(...)` input keys, reporting via `buildDescription(...)`. + +## The load-bearing rule: abstain, never false-positive + +The check only reasons about statically-resolvable values. The instant anything is genuinely dynamic — an operation +name from a field or parameter, a model passed in from elsewhere, `addImport(someUrl)` — it returns +`Description.NO_MATCH` and never errors. `DynamicClient` exists *for* runtime dynamism; a checker that flags valid +dynamic code gets turned off. It catches typos and stays out of the way. (See `abstainsOnDynamicOperationName` and +`abstainsWhenModelNotStaticallyResolvable` in the tests, and the CLI's `SmithyCall`, which is entirely in the abstain +set.) + +## Running it in a consumer build + +With the `net.ltgt.errorprone` Gradle plugin, put this module on the `errorprone` configuration: + +```kotlin +plugins { id("net.ltgt.errorprone") version "..." } +dependencies { + errorprone("com.google.errorprone:error_prone_core:2.50.0") + errorprone(project(":client:dynamic-client-errorprone")) +} +``` + +### Running *only* this check (no other Error Prone checks) + +Consumers who don't want Error Prone's ~500 built-in checks can disable them all and enable just this one: + +```kotlin +tasks.withType().configureEach { + options.errorprone { + disableAllChecks = true + error("DynamicClientUsage") + } +} +``` + +Note this still requires Error Prone the framework to be present — see "Known limitations". + +## Try it + +``` +./gradlew :client:dynamic-client-errorprone:test +``` + +The `CompilationTestHelper` tests compile inline sources against the real `DynamicClient` with the check active and +assert the diagnostics (unknown operation, bad input key, auto-detected service, and correct abstention on dynamic op +name and unresolved model). + +## Java 25 / toolchain note + +Error Prone runs inside `javac` and depends on compiler internals, so it needs the standard `--add-exports` / +`--add-opens` set and `--should-stop=ifError=FLOW`. Verified working on this project's **Java 25** toolchain with +Error Prone **2.50.0**. Because `--add-exports` of a system-module package is incompatible with `--release`, this +module compiles with plain `source`/`target` 21 rather than the `--release 21` the shared conventions set (see +`build.gradle.kts`). + +## Known limitations / next steps + +- **Requires Error Prone the framework.** "Only this check" disables the *other checks*, but consumers still adopt + Error Prone + `net.ltgt.errorprone`. If a zero-Error-Prone dependency is required, the raw-javac-plugin form (in git + history) or a standalone parser-based Gradle task are the alternatives; both reuse the same `ModelResolver` core. +- **Coupled to javac / Error Prone versions.** Newer JDKs can require a matching Error Prone bump. +- **Intra-unit flow only.** A client built in one file and used in another isn't tracked; an optional + `@SmithyModel(...)` fallback annotation naming the source would extend reach. +- **`discoverModels()` / classpath imports** are approximated via import roots; wiring the compilation's real + `JavaFileManager` classpath would make it exact. +- Only operation names and `Map.of` input keys are checked today; required members, enum values, and nested document + shapes are natural extensions. +``` diff --git a/client/dynamic-client-errorprone/build.gradle.kts b/client/dynamic-client-errorprone/build.gradle.kts new file mode 100644 index 000000000..68fd3ec1d --- /dev/null +++ b/client/dynamic-client-errorprone/build.gradle.kts @@ -0,0 +1,72 @@ +plugins { + id("smithy-java.module-conventions") +} + +description = "Error Prone check that validates DynamicClient usage against the Smithy model at compile time" + +extra["displayName"] = "Smithy :: Java :: Dynamic client Error Prone check" +extra["moduleName"] = "software.amazon.smithy.java.dynamicclient.compiler" + +dependencies { + // The Error Prone check API this BugChecker extends. + compileOnly(libs.errorprone.check.api) + compileOnly(libs.errorprone.annotation) + + // AutoService generates the META-INF/services/com.google.errorprone.bugpatterns.BugChecker registration. + compileOnly("com.google.auto.service:auto-service-annotations:1.1.1") + annotationProcessor("com.google.auto.service:auto-service:1.1.1") + + // The check runs inside javac and assembles the resolved model with the same ModelAssembler the runtime uses. + implementation(libs.smithy.model) + + testImplementation(libs.errorprone.test.helpers) + testImplementation(libs.errorprone.core) + testImplementation(project(":client:dynamic-client")) +} + +// Error Prone's check API compiles against javac internals, which require these exports on JDK 16+. +// `--add-exports` of a system-module package is incompatible with `--release`, so this module compiles with +// plain source/target (21, matching its dependencies) instead of the `--release 21` the conventions set. +tasks.withType().configureEach { + options.release.set(null as Int?) + sourceCompatibility = "21" + targetCompatibility = "21" + options.compilerArgs.addAll( + listOf( + "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + ), + ) +} + +// Javadoc also parses the compiler-internal references, so it needs the same exports. +tasks.withType().configureEach { + (options as StandardJavadocDocletOptions).addMultilineStringsOption("-add-exports").value = listOf( + "jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED", + "jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + ) +} + +// SpotBugs analyzes bytecode that references javac internals it cannot load; not meaningful for an Error Prone check. +tasks.named("spotbugsMain") { enabled = false } + +// The Error Prone test helpers spin up an in-process javac that also needs the module opens/exports at runtime. +tasks.withType().configureEach { + jvmArgs( + "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED", + "--add-exports=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED", + "--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED", + "--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED", + ) +} diff --git a/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DeclarationFinder.java b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DeclarationFinder.java new file mode 100644 index 000000000..411d05e1c --- /dev/null +++ b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DeclarationFinder.java @@ -0,0 +1,42 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import com.google.errorprone.VisitorState; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.TreePathScanner; +import com.sun.tools.javac.code.Symbol; + +/** + * Scans a compilation unit for the {@link VariableTree} that declares a given {@link Symbol}. Used as a fallback when + * the declaration is not on the path from the current call site to the top level (e.g. a field, or a local declared in + * a sibling method within the same file). + */ +final class DeclarationFinder { + + private DeclarationFinder() {} + + static VariableTree find(Symbol target, VisitorState state) { + var unit = state.getPath().getCompilationUnit(); + if (unit == null) { + return null; + } + var scanner = new TreePathScanner() { + VariableTree found; + + @Override + public Void visitVariable(VariableTree node, Void unused) { + if (found == null && ASTHelpers.getSymbol(node) == target) { + found = node; + } + return super.visitVariable(node, unused); + } + }; + scanner.scan(unit, null); + return scanner.found; + } +} diff --git a/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageChecker.java b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageChecker.java new file mode 100644 index 000000000..786d1c92a --- /dev/null +++ b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageChecker.java @@ -0,0 +1,313 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import com.google.auto.service.AutoService; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker; +import com.google.errorprone.fixes.SuggestedFix; +import com.google.errorprone.matchers.Description; +import com.google.errorprone.matchers.Matcher; +import com.google.errorprone.matchers.method.MethodMatchers; +import com.google.errorprone.util.ASTHelpers; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import com.sun.source.tree.VariableTree; +import com.sun.tools.javac.code.Symbol; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.shapes.MemberShape; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.StructureShape; + +/** + * Error Prone check that validates {@code DynamicClient} usage against the Smithy model each client is actually built + * from — at compile time, with no code generation. + * + *

The dynamic client stays fully runtime-driven and document-based. This check only reads the source under + * compilation: it finds {@code DynamicClient.builder()...build()} chains, resolves the model source by statically + * walking the {@code Model.assembler()...} chain feeding {@code .model(...)}, assembles that model in-process with the + * real {@code smithy-model} {@link software.amazon.smithy.model.loader.ModelAssembler}, and checks each + * {@code client.call("Op", Map.of(...))} site: + * + *

    + *
  • the operation name (a String literal or {@code static final String} constant) exists on the resolved + * service; and
    • + *
  • if the input is a {@code Map.of("k", v, ...)} literal, every key is a member of the operation input.
    • + *
+ * + *

The load-bearing rule: abstain, never false-positive

+ * + *

The check only reasons about statically-resolvable values. The instant a value is genuinely dynamic — an + * operation name from a field or CLI arg, a model path computed at runtime, {@code addImport(someUrl)} — it returns + * {@link Description#NO_MATCH}. The whole reason {@code DynamicClient} exists is runtime dynamism; a checker that flags + * valid dynamic code gets turned off. It catches typos and stays out of the way otherwise. + * + *

Running only this check

+ * + *

Consumers who do not want Error Prone's other checks can run this one alone: + * {@code -Xep:DynamicClientUsage:ERROR -XepDisableAllChecks}. + */ +@AutoService(BugChecker.class) +@BugPattern( + name = "DynamicClientUsage", + summary = "Verifies DynamicClient operation names and input keys against the Smithy model the client is " + + "built from.", + severity = BugPattern.SeverityLevel.ERROR, + link = "https://github.com/smithy-lang/smithy-java", + linkType = BugPattern.LinkType.CUSTOM) +public final class DynamicClientUsageChecker extends BugChecker implements BugChecker.MethodInvocationTreeMatcher { + + private static final String DYNAMIC_CLIENT = "software.amazon.smithy.java.dynamicclient.DynamicClient"; + + /** Matches any {@code call(...)} instance method on DynamicClient (all overloads take the op name first). */ + private static final Matcher CALL = + MethodMatchers.instanceMethod().onExactClass(DYNAMIC_CLIENT).named("call"); + + /** + * Cache of resolved clients keyed by the builder variable's symbol, so the model for a given client is assembled + * once per compilation regardless of how many call sites reference it. Keyed by symbol identity via its string + * form; a fresh resolver-backed model is cheap to look up but not to assemble. + */ + private final Map clientCache = new ConcurrentHashMap<>(); + + @Override + public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { + if (!CALL.matches(tree, state)) { + return Description.NO_MATCH; + } + var args = tree.getArguments(); + if (args.isEmpty()) { + return Description.NO_MATCH; + } + + // Resolve which client this call is made on, and the model it was built from. + ResolvedClient client = resolveClient(tree, state); + if (client == null || client.model() == null) { + return Description.NO_MATCH; // unknown client or unresolved model: abstain. + } + + // Operation name must be a compile-time constant to validate. ASTHelpers.constValue folds literals + constants. + String operation = ASTHelpers.constValue(args.get(0), String.class); + if (operation == null) { + return Description.NO_MATCH; // dynamic operation name: abstain. + } + + OperationShape opShape = client.operations().get(operation); + if (opShape == null) { + return buildUnknownOperation(tree, args.get(0), operation, client, state); + } + + // If a second argument is a Map.of(...) literal, validate its keys against the input structure members. + if (args.size() >= 2) { + Description inputProblem = validateInputKeys(args.get(1), opShape, client); + if (inputProblem != null) { + return inputProblem; + } + } + return Description.NO_MATCH; + } + + private Description buildUnknownOperation( + MethodInvocationTree tree, + ExpressionTree opArg, + String operation, + ResolvedClient client, + VisitorState state + ) { + var description = buildDescription(opArg) + .setMessage(String.format( + "Operation '%s' not found in service '%s'. Known operations: %s", + operation, + client.service().getId(), + client.sortedOperationNames())); + // Suggested fix: if there is a single close match by edit distance, offer it. (Auto-fix is EP's payoff.) + String suggestion = closestMatch(operation, client.operations().keySet()); + if (suggestion != null) { + description.addFix(SuggestedFix.replace(opArg, '"' + suggestion + '"')); + } + return description.build(); + } + + private Description validateInputKeys(ExpressionTree inputArg, OperationShape opShape, ResolvedClient client) { + List keys = MapLiteral.keysOf(inputArg, e -> ASTHelpers.constValue(e, String.class)); + if (keys == null) { + return null; // not a resolvable Map.of(...) literal (e.g. a Document or variable): nothing to check. + } + var inputId = opShape.getInputShape(); + var inputShape = client.model().getShape(inputId).orElse(null); + if (!(inputShape instanceof StructureShape structure)) { + return null; + } + Map members = structure.getAllMembers(); + for (String key : keys) { + if (!members.containsKey(key)) { + return buildDescription(inputArg) + .setMessage(String.format( + "'%s' is not a member of input '%s' for operation '%s'. Known members: %s", + key, + inputId.getName(), + opShape.getId().getName(), + members.keySet().stream().sorted().toList())) + .build(); + } + } + return null; + } + + /** Resolve the receiver of a {@code call(...)} back to the {@code DynamicClient} builder that produced it. */ + private ResolvedClient resolveClient(MethodInvocationTree call, VisitorState state) { + ExpressionTree receiver = ASTHelpers.getReceiver(call); + if (receiver == null) { + return null; + } + Symbol receiverSymbol = ASTHelpers.getSymbol(receiver); + if (receiverSymbol == null) { + return null; + } + ResolvedClient cached = clientCache.get(receiverSymbol); + if (cached != null) { + return cached; + } + // Find the variable declaration for the receiver and inspect its initializer builder chain. + VariableTree declaration = findDeclaration(receiverSymbol, state); + if (declaration == null || declaration.getInitializer() == null) { + return null; + } + if (!isDynamicClientBuild(declaration.getInitializer())) { + return null; + } + ExpressionTree modelArg = findNamedArgument(declaration.getInitializer(), "model"); + ExpressionTree modelExpr = modelArg == null ? null : deref(modelArg, state); + Model model = modelExpr == null + ? null + : new ModelResolver(e -> ASTHelpers.constValue(e, String.class), List.of()).resolve(modelExpr); + ExpressionTree serviceArg = findNamedArgument(declaration.getInitializer(), "serviceId"); + String serviceId = resolveServiceId(serviceArg); + ResolvedClient resolved = ResolvedClient.of(model, serviceId); + clientCache.put(receiverSymbol, resolved); + return resolved; + } + + /** + * Resolve the service ID from a {@code serviceId(...)} argument. Handles both a String constant and the common + * {@code ShapeId.from("ns#Name")} form. Returns {@code null} when not statically resolvable, in which case + * {@link ResolvedClient} falls back to single-service auto-detection like the runtime builder does. + */ + private String resolveServiceId(ExpressionTree serviceArg) { + if (serviceArg == null) { + return null; + } + String direct = ASTHelpers.constValue(serviceArg, String.class); + if (direct != null) { + return direct; + } + // ShapeId.from("smithy.example#Sprockets") + if (serviceArg instanceof MethodInvocationTree mi + && mi.getMethodSelect() instanceof MemberSelectTree sel + && sel.getIdentifier().contentEquals("from") + && sel.getExpression().toString().endsWith("ShapeId") + && mi.getArguments().size() == 1) { + return ASTHelpers.constValue(mi.getArguments().get(0), String.class); + } + return null; + } + + /** Whether an initializer is a {@code DynamicClient.builder()...build()} chain. */ + private boolean isDynamicClientBuild(ExpressionTree expr) { + ExpressionTree cursor = expr; + while (cursor instanceof MethodInvocationTree mi + && mi.getMethodSelect() instanceof MemberSelectTree sel) { + if (sel.getIdentifier().contentEquals("builder") + && sel.getExpression().toString().endsWith("DynamicClient")) { + return true; + } + cursor = sel.getExpression(); + } + return false; + } + + /** Find the argument passed to {@code .name(x)} anywhere in a builder chain. */ + private ExpressionTree findNamedArgument(ExpressionTree expr, String methodName) { + ExpressionTree cursor = expr; + while (cursor instanceof MethodInvocationTree mi + && mi.getMethodSelect() instanceof MemberSelectTree sel) { + if (sel.getIdentifier().contentEquals(methodName) && !mi.getArguments().isEmpty()) { + return mi.getArguments().get(0); + } + cursor = sel.getExpression(); + } + return null; + } + + /** If the model argument is an identifier, dereference it to the initializer of the variable it names. */ + private ExpressionTree deref(ExpressionTree expr, VisitorState state) { + if (expr instanceof MethodInvocationTree) { + return expr; // already a Model.assembler()... chain. + } + Symbol symbol = ASTHelpers.getSymbol(expr); + if (symbol == null) { + return expr; + } + VariableTree declaration = findDeclaration(symbol, state); + if (declaration != null && declaration.getInitializer() != null) { + return declaration.getInitializer(); + } + return expr; + } + + /** Locate the {@link VariableTree} declaring {@code symbol} within the current compilation unit. */ + private VariableTree findDeclaration(Symbol symbol, VisitorState state) { + return DeclarationFinder.find(symbol, state); + } + + /** Return the single closest operation name by Levenshtein distance if it is unambiguously close, else null. */ + private static String closestMatch(String typo, Set candidates) { + String best = null; + int bestDistance = Integer.MAX_VALUE; + int secondBest = Integer.MAX_VALUE; + for (String candidate : candidates) { + int distance = levenshtein(typo, candidate); + if (distance < bestDistance) { + secondBest = bestDistance; + bestDistance = distance; + best = candidate; + } else if (distance < secondBest) { + secondBest = distance; + } + } + // Only suggest when there's a clear, close winner (<= a third of the length, and distinctly closest). + int threshold = Math.max(1, typo.length() / 3); + if (best != null && bestDistance <= threshold && bestDistance < secondBest) { + return best; + } + return null; + } + + private static int levenshtein(String a, String b) { + int[] prev = new int[b.length() + 1]; + int[] curr = new int[b.length() + 1]; + for (int j = 0; j <= b.length(); j++) { + prev[j] = j; + } + for (int i = 1; i <= a.length(); i++) { + curr[0] = i; + for (int j = 1; j <= b.length(); j++) { + int cost = a.charAt(i - 1) == b.charAt(j - 1) ? 0 : 1; + curr[j] = Math.min(Math.min(curr[j - 1] + 1, prev[j] + 1), prev[j - 1] + cost); + } + int[] tmp = prev; + prev = curr; + curr = tmp; + } + return prev[b.length()]; + } +} diff --git a/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/MapLiteral.java b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/MapLiteral.java new file mode 100644 index 000000000..0509b35fc --- /dev/null +++ b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/MapLiteral.java @@ -0,0 +1,48 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Function; + +/** + * Extracts the string keys from a {@code Map.of("k1", v1, "k2", v2, ...)} or {@code Map.ofEntries(...)}-free literal + * so input members can be checked at compile time. + */ +final class MapLiteral { + + private MapLiteral() {} + + /** + * @return the list of literal string keys if {@code expr} is a fully-resolvable {@code Map.of(...)} call with an + * even argument count and constant keys; otherwise {@code null} (meaning "not a checkable map literal"). + */ + static List keysOf(ExpressionTree expr, Function constantResolver) { + if (!(expr instanceof MethodInvocationTree call) + || !(call.getMethodSelect() instanceof MemberSelectTree select) + || !select.getIdentifier().contentEquals("of") + || !select.getExpression().toString().endsWith("Map")) { + return null; + } + var args = call.getArguments(); + if (args.isEmpty() || args.size() % 2 != 0) { + return null; // Map.of() empty is fine (no keys to check) but returns empty below; odd => not Map.of(k,v..). + } + List keys = new ArrayList<>(); + for (int i = 0; i < args.size(); i += 2) { + String key = constantResolver.apply(args.get(i)); + if (key == null) { + return null; // a non-constant key: give up on key checking rather than half-check. + } + keys.add(key); + } + return keys; + } +} diff --git a/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ModelResolver.java b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ModelResolver.java new file mode 100644 index 000000000..b10e83591 --- /dev/null +++ b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ModelResolver.java @@ -0,0 +1,196 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.MethodInvocationTree; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.loader.ModelAssembler; +import software.amazon.smithy.model.validation.Severity; + +/** + * Resolves the Smithy {@link Model} that a {@code DynamicClient} is built from by statically reading the + * {@code Model.assembler()...assemble().unwrap()} chain that feeds {@code builder().model(...)}. + * + *

This is the mechanism-agnostic core of the check: static source resolution, then compile-time assembly using the + * exact same {@link ModelAssembler} the runtime uses (no reimplementation, no drift). It has no dependency on javac + * internals or Error Prone — the caller supplies a {@code constantResolver} that maps an expression to its compile-time + * String value (backed by {@code ASTHelpers.constValue} under Error Prone), so the same resolver drives an Error Prone + * {@code BugChecker}, a raw javac plugin, or a standalone parser-based task unchanged. + * + *

Every resolvable chain is assembled at most once and cached, keyed by the ordered set of sources it declares. If + * any source-adding call in the chain has an argument the resolver cannot reduce to a compile-time constant, the whole + * model is treated as unresolvable and {@link #resolve} returns {@code null} — the caller then abstains rather than + * validating against a partial model. + */ +final class ModelResolver { + + private final Function constantResolver; + private final List importRoots; + private final Map cache = new LinkedHashMap<>(); + + /** + * @param constantResolver maps an expression to its compile-time String value, or {@code null} if it is not a + * compile-time constant (literals and {@code static final String}s included). + * @param importRoots directories to resolve relative {@code addImport("path")} arguments against. + */ + ModelResolver(Function constantResolver, List importRoots) { + this.constantResolver = constantResolver; + this.importRoots = importRoots; + } + + /** + * Attempt to resolve and assemble the model from the expression passed to {@code .model(expr)}. + * + * @param modelExpr the argument expression given to {@code .model(...)}, already dereferenced to its defining + * initializer where possible. + * @return the assembled model, or {@code null} if the chain is not fully statically resolvable. + */ + Model resolve(ExpressionTree modelExpr) { + if (!(modelExpr instanceof MethodInvocationTree)) { + return null; // e.g. a bare identifier we could not dereference, or a method return: abstain. + } + // Collect the assembler chain sources from the outermost call (unwrap/assemble) inward to assembler(). + var sources = new AssemblerSources(); + if (!collectChain((MethodInvocationTree) modelExpr, sources)) { + return null; // encountered a non-resolvable source or an unrecognized chain: abstain. + } + if (!sources.sawAssembler) { + return null; // not actually a Model.assembler() chain. + } + String key = sources.cacheKey(); + Model cached = cache.get(key); + if (cached != null) { + return cached; + } + Model assembled = assemble(sources); + if (assembled != null) { + cache.put(key, assembled); + } + return assembled; + } + + /** Walk a fluent chain of method invocations, gathering statically-known model sources. */ + private boolean collectChain(MethodInvocationTree invocation, AssemblerSources sources) { + if (!(invocation.getMethodSelect() instanceof MemberSelectTree select)) { + return false; + } + String method = select.getIdentifier().toString(); + ExpressionTree receiver = select.getExpression(); + + switch (method) { + case "unwrap", "assemble", "putProperty" -> { + // Structural / no-op-for-us calls; recurse into the receiver. + } + case "discoverModels" -> sources.discoverModels = true; + case "addUnparsedModel" -> { + // addUnparsedModel(String name, String content) + var args = invocation.getArguments(); + if (args.size() != 2) { + return false; + } + String name = constantResolver.apply(args.get(0)); + String content = constantResolver.apply(args.get(1)); + if (name == null || content == null) { + return false; // dynamic content: abstain. + } + sources.unparsed.put(name, content); + } + case "addImport" -> { + // addImport(String path) with a literal path. addImport(URL)/computed paths => abstain. + var args = invocation.getArguments(); + if (args.size() != 1) { + return false; + } + String path = constantResolver.apply(args.get(0)); + if (path == null) { + return false; // e.g. addImport(resourceUrl): abstain. + } + sources.imports.add(path); + } + case "assembler" -> { + // Model.assembler() — the root. Stop here. + sources.sawAssembler = true; + return true; + } + default -> { + // Unknown builder method in the chain: be conservative and abstain. + return false; + } + } + + if (receiver instanceof MethodInvocationTree next) { + return collectChain(next, sources); + } + // Reached the head of the chain without seeing assembler() (e.g. Model.assembler where assembler is a field). + return method.equals("assembler"); + } + + private Model assemble(AssemblerSources sources) { + try { + ModelAssembler assembler = Model.assembler() + .putProperty(ModelAssembler.ALLOW_UNKNOWN_TRAITS, true); + if (sources.discoverModels) { + assembler.discoverModels(); + } + for (var entry : sources.unparsed.entrySet()) { + assembler.addUnparsedModel(entry.getKey(), entry.getValue()); + } + for (String imp : sources.imports) { + Path resolved = resolveImportPath(imp); + if (resolved == null) { + // Could not locate a literal import against any known root: abstain rather than assemble partial. + return null; + } + assembler.addImport(resolved); + } + var result = assembler.assemble(); + if (result.getValidationEvents() + .stream() + .anyMatch(ev -> ev.getSeverity() == Severity.ERROR)) { + // The user's model itself doesn't assemble cleanly; that's their build's job to report, not ours. + return null; + } + return result.unwrap(); + } catch (RuntimeException e) { + // Any assembly failure => we cannot validate; abstain silently. + return null; + } + } + + private Path resolveImportPath(String imp) { + Path direct = Path.of(imp); + if (direct.toFile().exists()) { + return direct; + } + for (String root : importRoots) { + Path candidate = Path.of(root).resolve(imp); + if (candidate.toFile().exists()) { + return candidate; + } + } + return null; + } + + /** Ordered, statically-known sources declared by an assembler chain. */ + private static final class AssemblerSources { + private final Map unparsed = new LinkedHashMap<>(); + private final List imports = new ArrayList<>(); + private boolean discoverModels; + private boolean sawAssembler; + + String cacheKey() { + return "discover=" + discoverModels + ";unparsed=" + unparsed + ";imports=" + imports; + } + } +} diff --git a/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ResolvedClient.java b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ResolvedClient.java new file mode 100644 index 000000000..54ebf4e95 --- /dev/null +++ b/client/dynamic-client-errorprone/src/main/java/software/amazon/smithy/java/dynamicclient/compiler/ResolvedClient.java @@ -0,0 +1,83 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import software.amazon.smithy.model.Model; +import software.amazon.smithy.model.knowledge.TopDownIndex; +import software.amazon.smithy.model.shapes.OperationShape; +import software.amazon.smithy.model.shapes.ServiceShape; +import software.amazon.smithy.model.shapes.ShapeId; + +/** + * The resolution result for one {@code DynamicClient} variable: the model it is built from and the service selected on + * it, plus a precomputed operation-name lookup mirroring {@code DynamicClient}'s own runtime table. + */ +final class ResolvedClient { + + private final Model model; + private final ServiceShape service; + private final Map operations; + + private ResolvedClient(Model model, ServiceShape service, Map operations) { + this.model = model; + this.service = service; + this.operations = operations; + } + + /** + * Build a resolved client from a model and an optional explicit service ID. Mirrors {@code DynamicClient.Builder}: + * an explicit service is used if present; otherwise a single service in the model is auto-detected. If the service + * cannot be determined (zero or multiple, none specified), returns a client with a null model so callers abstain. + */ + static ResolvedClient of(Model model, String serviceIdLiteral) { + if (model == null) { + return new ResolvedClient(null, null, Map.of()); + } + ServiceShape service = selectService(model, serviceIdLiteral); + if (service == null) { + // Same abstain-if-ambiguous stance the runtime takes by throwing; here we simply can't validate. + return new ResolvedClient(null, null, Map.of()); + } + Map ops = new LinkedHashMap<>(); + for (OperationShape op : TopDownIndex.of(model).getContainedOperations(service)) { + ops.put(op.getId().getName(), op); + } + return new ResolvedClient(model, service, ops); + } + + private static ServiceShape selectService(Model model, String serviceIdLiteral) { + if (serviceIdLiteral != null) { + try { + return model.getShape(ShapeId.from(serviceIdLiteral)) + .flatMap(s -> s.asServiceShape()) + .orElse(null); + } catch (RuntimeException e) { + return null; + } + } + var services = model.getServiceShapes(); + return services.size() == 1 ? services.iterator().next() : null; + } + + Model model() { + return model; + } + + ServiceShape service() { + return service; + } + + Map operations() { + return operations; + } + + List sortedOperationNames() { + return operations.keySet().stream().sorted().toList(); + } +} diff --git a/client/dynamic-client-errorprone/src/test/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageCheckerTest.java b/client/dynamic-client-errorprone/src/test/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageCheckerTest.java new file mode 100644 index 000000000..7fe557b7c --- /dev/null +++ b/client/dynamic-client-errorprone/src/test/java/software/amazon/smithy/java/dynamicclient/compiler/DynamicClientUsageCheckerTest.java @@ -0,0 +1,166 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +package software.amazon.smithy.java.dynamicclient.compiler; + +import com.google.errorprone.CompilationTestHelper; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link DynamicClientUsageChecker} using Error Prone's {@link CompilationTestHelper}, which compiles the + * inline sources against the real {@code DynamicClient} with the check active and asserts on the diagnostics via + * {@code // BUG: Diagnostic contains:} markers. This replaces the raw-plugin prototype's shell driver. + */ +class DynamicClientUsageCheckerTest { + + private CompilationTestHelper helper() { + return CompilationTestHelper.newInstance(DynamicClientUsageChecker.class, getClass()); + } + + // A single-line Smithy model embedded as a Java string constant (newlines escaped for the source under test). + private static final String MODEL = String.join( + "\\n", + "$version: \\\"2\\\"", + "namespace smithy.example", + "service Sprockets { operations: [CreateSprocket, GetSprocket] }", + "operation CreateSprocket { input := {} output := {} }", + "operation GetSprocket { input := { id: String } output := { id: String } }"); + + @Test + void flagsUnknownOperation() { + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "import software.amazon.smithy.model.shapes.ShapeId;", + "class Demo {", + " static final String MODEL = \"" + MODEL + "\";", + " void run() {", + " DynamicClient client = DynamicClient.builder()", + " .serviceId(ShapeId.from(\"smithy.example#Sprockets\"))", + " .model(Model.assembler().addUnparsedModel(\"demo.smithy\", MODEL).assemble().unwrap())", + " .build();", + " // BUG: Diagnostic contains: Operation 'GetSprokcet' not found", + " client.call(\"GetSprokcet\", Map.of(\"id\", \"1\"));", + " }", + "}") + .doTest(); + } + + @Test + void flagsUnknownInputMember() { + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "import software.amazon.smithy.model.shapes.ShapeId;", + "class Demo {", + " static final String MODEL = \"" + MODEL + "\";", + " void run() {", + " DynamicClient client = DynamicClient.builder()", + " .serviceId(ShapeId.from(\"smithy.example#Sprockets\"))", + " .model(Model.assembler().addUnparsedModel(\"demo.smithy\", MODEL).assemble().unwrap())", + " .build();", + " // BUG: Diagnostic contains: 'idd' is not a member of input 'GetSprocketInput'", + " client.call(\"GetSprocket\", Map.of(\"idd\", \"1\"));", + " }", + "}") + .doTest(); + } + + @Test + void acceptsValidCalls() { + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "import software.amazon.smithy.model.shapes.ShapeId;", + "class Demo {", + " static final String MODEL = \"" + MODEL + "\";", + " void run() {", + " DynamicClient client = DynamicClient.builder()", + " .serviceId(ShapeId.from(\"smithy.example#Sprockets\"))", + " .model(Model.assembler().addUnparsedModel(\"demo.smithy\", MODEL).assemble().unwrap())", + " .build();", + " client.call(\"GetSprocket\", Map.of(\"id\", \"1\"));", + " client.call(\"CreateSprocket\");", + " }", + "}") + .doTest(); + } + + @Test + void detectsServiceWhenNoServiceIdGiven() { + // Single service in the model: the checker auto-detects it, mirroring the runtime builder. + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "class Demo {", + " static final String MODEL = \"" + MODEL + "\";", + " void run() {", + " DynamicClient client = DynamicClient.builder()", + " .model(Model.assembler().addUnparsedModel(\"demo.smithy\", MODEL).assemble().unwrap())", + " .build();", + " // BUG: Diagnostic contains: Operation 'Nope' not found", + " client.call(\"Nope\");", + " }", + "}") + .doTest(); + } + + @Test + void abstainsOnDynamicOperationName() { + // Operation name comes from a parameter: the check must NOT flag it. + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "import software.amazon.smithy.model.shapes.ShapeId;", + "class Demo {", + " static final String MODEL = \"" + MODEL + "\";", + " void run(String op) {", + " DynamicClient client = DynamicClient.builder()", + " .serviceId(ShapeId.from(\"smithy.example#Sprockets\"))", + " .model(Model.assembler().addUnparsedModel(\"demo.smithy\", MODEL).assemble().unwrap())", + " .build();", + " client.call(op, Map.of(\"id\", \"1\"));", + " }", + "}") + .doTest(); + } + + @Test + void abstainsWhenModelNotStaticallyResolvable() { + // Model is passed in as a parameter: nothing to resolve, so no flags even for a bogus operation. + helper() + .addSourceLines( + "Demo.java", + "import java.util.Map;", + "import software.amazon.smithy.java.dynamicclient.DynamicClient;", + "import software.amazon.smithy.model.Model;", + "import software.amazon.smithy.model.shapes.ShapeId;", + "class Demo {", + " void run(Model model) {", + " DynamicClient client = DynamicClient.builder()", + " .serviceId(ShapeId.from(\"smithy.example#Sprockets\"))", + " .model(model)", + " .build();", + " client.call(\"TotallyNotAnOperation\", Map.of(\"id\", \"1\"));", + " }", + "}") + .doTest(); + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 6a6796c60..13b4ade0b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -29,8 +29,13 @@ jspecify = "1.0.0" commonmark = "0.29.0" jsoup = "1.22.2" aws-api-models = "1.0.269" +errorprone = "2.50.0" [libraries] +errorprone-check-api = { module = "com.google.errorprone:error_prone_check_api", version.ref = "errorprone" } +errorprone-core = { module = "com.google.errorprone:error_prone_core", version.ref = "errorprone" } +errorprone-annotation = { module = "com.google.errorprone:error_prone_annotation", version.ref = "errorprone" } +errorprone-test-helpers = { module = "com.google.errorprone:error_prone_test_helpers", version.ref = "errorprone" } smithy-model = { module = "software.amazon.smithy:smithy-model", version.ref = "smithy" } smithy-codegen = { module = "software.amazon.smithy:smithy-codegen-core", version.ref = "smithy" } smithy-aws-traits = { module = "software.amazon.smithy:smithy-aws-traits", version.ref = "smithy" } diff --git a/settings.gradle.kts b/settings.gradle.kts index 0e57486b3..bf5fe2813 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -53,6 +53,7 @@ include(":client:client-http-boringssl") include(":client:client-rpcv2-cbor") include(":client:client-rpcv2-json") include(":client:dynamic-client") +include(":client:dynamic-client-errorprone") include(":client:client-mock-plugin") include(":client:client-waiters") include(":client:client-rulesengine")