Accept any annotation with simple name Contract, and change reporting of invalid contract annotations#1295
Accept any annotation with simple name Contract, and change reporting of invalid contract annotations#1295
Contract, and change reporting of invalid contract annotations#1295Conversation
WalkthroughDeclaration-time @contract parsing now validates antecedent value-constraints and reports unknown values; dataflow no longer emits ANNOTATION_VALUE_INVALID and instead marks clauses unsupported; utilities gate invalid-contract diagnostics to MethodTree and recognize simple-name Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Utils as ContractUtils
participant Decl as ContractCheckHandler
participant DF as ContractHandler
participant Compiler
Compiler->>Utils: parse @Contract (report only for MethodTree)
Utils-->>Compiler: clauses or parse-error marker
Compiler->>Decl: onMatchMethod(clauses) [declaration-time]
rect rgb(209,241,209)
note right of Decl: For each antecedent value-constraint:\n- unknown -> emit ANNOTATION_VALUE_INVALID\n- known & non-checkable -> mark unsupported (stop body checks)\n- checkable -> continue validation
end
Decl-->>Compiler: declaration diagnostics
Compiler->>DF: onDataflowVisitMethodInvocation(clauses) [call-site]
rect rgb(241,226,226)
note right of DF: Invalid clauses are NOT reported here\n—they are marked unsupported instead
end
DF-->>Compiler: dataflow diagnostics (no annotation-value errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1295 +/- ##
============================================
+ Coverage 88.38% 88.47% +0.09%
- Complexity 2468 2476 +8
============================================
Files 93 93
Lines 8191 8203 +12
Branches 1615 1616 +1
============================================
+ Hits 7240 7258 +18
+ Misses 478 474 -4
+ Partials 473 471 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
nullaway/src/test/java/com/uber/nullaway/handlers/contract/ContractUtilsTest.java (1)
11-11: Config import added but not used in meaningful tests.The
Configclass is imported and mocked, but it's only used inverifyNoInteractions. SinceContractUtils.getAntecedentdoesn't actually interact withConfig(based on the code inContractUtils.java), this mock doesn't test any real behavior. Theconfigparameter appears to be passed to other methods likegetContractStringandgetContractClauses, notgetAntecedent.Consider either:
- Remove the unused
Configmock if it's not needed for testinggetAntecedent- Add tests for methods that actually use
Config(e.g.,getContractString,getContractClauses)-import com.uber.nullaway.Config; import com.uber.nullaway.NullAway; import org.junit.Before; import org.junit.Test; public class ContractUtilsTest { private Tree tree; private NullAway analysis; private VisitorState state; private Symbol symbol; - private Config config; @Before public void setUp() { tree = mock(Tree.class); analysis = mock(NullAway.class, RETURNS_MOCKS); state = mock(VisitorState.class); symbol = mock(Symbol.class); - config = mock(Config.class); } @Test public void getEmptyAntecedent() { String[] antecedent = ContractUtils.getAntecedent("->_", tree, analysis, state, symbol, 0); assertArrayEquals(new String[0], antecedent); - verifyNoInteractions(tree, state, analysis, symbol, config); + verifyNoInteractions(tree, state, analysis, symbol); }Also applies to: 22-22, 30-30, 38-38
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java (1)
58-63: Consider using ImmutableSet for thread safety.Since these are constant sets that won't be modified after initialization, using
ImmutableSetfrom Guava would be safer and more explicit about their immutability.+import com.google.common.collect.ImmutableSet; import java.util.Set; public class ContractCheckHandler extends BaseNoOpHandler { private final Config config; /** A set of value constraints in the antecedent which we can check for now. */ - private final Set<String> checkableValueConstraints = Set.of("_", "null", "!null"); + private static final ImmutableSet<String> checkableValueConstraints = + ImmutableSet.of("_", "null", "!null"); /** All known valid value constraints */ - private final Set<String> allValidValueConstraints = - Set.of("_", "null", "!null", "true", "false"); + private static final ImmutableSet<String> allValidValueConstraints = + ImmutableSet.of("_", "null", "!null", "true", "false");nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (1)
143-149: Potential issue with edge case handling inendsWithContract.The method correctly handles most cases, but returns
falsewhen the input has no dots (simple name only). This means a simple annotation named "Contract" without a package would not be recognized.Consider handling the no-package case:
private static boolean endsWithContract(String input) { int lastDot = input.lastIndexOf('.'); if (lastDot == -1 || lastDot == input.length() - 1) { - return false; + // If no dot found, check if the entire string is "Contract" + // If dot is at the end, it's malformed + return lastDot == -1 ? input.equals("Contract") : false; } return input.substring(lastDot + 1).equals("Contract"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java(3 hunks)nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java(1 hunks)nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java(6 hunks)nullaway/src/test/java/com/uber/nullaway/ContractsTests.java(2 hunks)nullaway/src/test/java/com/uber/nullaway/handlers/contract/ContractUtilsTest.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
PR: uber/NullAway#1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/handlers/contract/ContractUtilsTest.javanullaway/src/test/java/com/uber/nullaway/ContractsTests.java
🧬 Code graph analysis (3)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java (1)
nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java (1)
ErrorMessage(26-73)
nullaway/src/test/java/com/uber/nullaway/handlers/contract/ContractUtilsTest.java (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (1)
ContractUtils(19-173)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (2)
nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java (1)
ErrorMessage(26-73)nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (1)
NullabilityUtil(66-665)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build caffeine with snapshot
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build and test on ubuntu-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build spring-framework with snapshot
🔇 Additional comments (5)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractHandler.java (1)
273-277: LGTM! Error reporting properly gated to method declarations.The change correctly suppresses invalid contract error reporting during dataflow analysis, preventing errors from appearing at call sites for library methods that users cannot modify.
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java (1)
93-112: LGTM! Comprehensive validation of contract value constraints.The validation logic properly distinguishes between invalid constraints (which trigger errors) and valid but unsupported constraints (which mark the contract as unsupported). The error messages are clear and include helpful documentation links.
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (1)
50-67: LGTM! Proper gating of error reporting to method declarations.The
shouldReportInvalidContracthelper correctly restricts error reporting toMethodTreeinstances, preventing spurious errors at call sites. BothgetConsequentandgetAntecedentproperly use this gate.Also applies to: 95-95, 170-172
nullaway/src/test/java/com/uber/nullaway/ContractsTests.java (2)
76-133: LGTM! Well-structured test for non-JetBrains Contract annotations.The test effectively validates that NullAway recognizes any annotation with simple name "Contract" as a contract annotation, while correctly ignoring "NotContract". The test cases properly verify both the positive cases (where contracts are honored) and negative cases (where non-contract annotations are ignored).
215-270: LGTM! Comprehensive testing of malformed contract validation.The test thoroughly validates error reporting for malformed contracts in both custom-named and built-in Contract annotations. It properly enables
CheckContracts=trueto trigger validation and verifies that errors are reported at method declarations, not at call sites.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (4)
3-3: Drop unnecessary Guava Function cast/import in trimReceivers.The explicit cast to Function is redundant; the lambda is type-inferred. Also remove the import.
-import com.google.common.base.Function; @@ - return fieldNames.stream() - .map((Function<String, String>) input -> input.substring(input.lastIndexOf(".") + 1)) - .collect(Collectors.toSet()); + return fieldNames.stream() + .map(input -> input.substring(input.lastIndexOf('.') + 1)) + .collect(Collectors.toSet());Also applies to: 29-33
95-107: Minor wording fix in error string.“arguments in for the method” → “arguments for the method”.
- + "arguments in for the method [" + + "arguments for the method ["
130-133: Rename helper for clarity: it checks simple-name equality, not “endsWith”.Method name endsWithContract suggests suffix-match; implementation checks simple-name equals "Contract". Rename for precision.
- if (endsWithContract(name) || config.isContractAnnotation(name)) { + if (hasSimpleNameContract(name) || config.isContractAnnotation(name)) { @@ - private static boolean endsWithContract(String input) { + private static boolean hasSimpleNameContract(String input) {Also applies to: 137-152
155-155: Nit: article before @contract.Prefer “an @contract annotation.”
- // Check to see if this method has a @Contract annotation + // Check to see if this method has an @Contract annotation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (2)
nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java (1)
ErrorMessage(26-73)nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (1)
NullabilityUtil(66-665)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build caffeine with snapshot
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build and test on ubuntu-latest
🔇 Additional comments (4)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (4)
5-5: LGTM: import aligns with new reporting guard.
18-18: LGTM: Javadoc grammar fix.
166-175: LGTM: reporting guard limited to MethodTree.This confines invalid-contract diagnostics to declarations.
50-67: Approve — errors now reported only on method declarations; tweak message spacing.shouldReportInvalidContract() is implemented as
tree instanceof MethodTree, and ContractCheckHandler is added only whenconfig.checkContracts()is true; the proposed spacing diff is correct.- + clause - + "(see https://www.jetbrains.com/help/idea/contract-annotations.html)."; + + clause + + " (see https://www.jetbrains.com/help/idea/contract-annotations.html).";
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (3)
28-32: Guard against trailing dots; avoid empty substrings.
If a field name ends with '.', this returns "". Minor but easy to harden.Apply this diff:
- return fieldNames.stream() - .map(input -> input.substring(input.lastIndexOf(".") + 1)) - .collect(Collectors.toSet()); + return fieldNames.stream() + .map(input -> { + int idx = input.lastIndexOf('.'); + return (idx >= 0 && idx < input.length() - 1) ? input.substring(idx + 1) : input; + }) + .collect(Collectors.toSet());
49-66: Good: restrict malformed-clause reporting to declaration sites. Minor copy edit in message.
Add a space before “(see …)”.- + clause - + "(see https://www.jetbrains.com/help/idea/contract-annotations.html)."; + + clause + + " (see https://www.jetbrains.com/help/idea/contract-annotations.html).";
136-152: Tiny simplification of simple-name check (optional).- private static boolean hasSimpleNameContract(String input) { - int lastDot = input.lastIndexOf('.'); - String simpleName; - if (lastDot == -1) { - simpleName = input; - } else { - simpleName = input.substring(lastDot + 1); - } - return simpleName.equals("Contract"); - } + private static boolean hasSimpleNameContract(String input) { + int i = input.lastIndexOf('.'); + String simple = (i >= 0) ? input.substring(i + 1) : input; + return simple.equals("Contract"); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (2)
nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java (1)
ErrorMessage(26-73)nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (1)
NullabilityUtil(66-665)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build and test on ubuntu-latest
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build caffeine with snapshot
🔇 Additional comments (4)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java (4)
4-4: Import is appropriate and used by new gating helper.
17-17: Javadoc grammar fix looks good.
94-104: Gating antecedent-size errors to MethodTree is correct.
165-174: invalid-contract reporting respects the CheckContracts flag
ContractCheckHandler is only registered in Handlers.java when config.checkContracts() is true, and the dataflow contract propagation in AccessPathNullnessAnalysis is likewise gated—so no invalid-contract reporting occurs if CheckContracts=false.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (2)
216-219: Good defensive fix; update Javadoc to reflect null-on-non-String behaviorNow safely handles non-String values. Please clarify the @return Javadoc to include the non-String case.
@@ - * @return The {@code value} attribute of the annotation, or {@code null} if the annotation is not - * present. + * @return The {@code value} attribute of the annotation, or {@code null} if the annotation is not + * present or the {@code value} is not a {@link String}.
249-253: Guard against non-String array elements to avoid ClassCastExceptionCurrent cast assumes all elements are Strings. Return null (like the single-value helper) if any element isn’t a String.
- @SuppressWarnings("unchecked") - List<AnnotationValue> values = (List<AnnotationValue>) entry.getValue().getValue(); - return values.stream().map((av) -> ((String) av.getValue())).collect(Collectors.toSet()); + @SuppressWarnings("unchecked") + List<AnnotationValue> values = (List<AnnotationValue>) entry.getValue().getValue(); + if (!values.stream().allMatch(av -> av.getValue() instanceof String)) { + return null; + } + return values.stream() + .map(av -> (String) av.getValue()) + .collect(Collectors.toSet());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java(1 hunks)nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractUtils.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
PR: uber/NullAway#1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build and test on ubuntu-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build caffeine with snapshot
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build spring-framework with snapshot
🔇 Additional comments (1)
nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java (1)
270-279: Simple-name match should be equality, not suffixUsing endsWith can misidentify annotations whose simple name merely ends with the token (e.g., FooContract). For correctness with “simple name Contract” semantics, compare the last segment.
- if ((exactMatch && name.equals(annotName)) || (!exactMatch && name.endsWith(annotName))) { + if (exactMatch + ? name.equals(annotName) + : name.substring(name.lastIndexOf('.') + 1).equals(annotName)) {Run to find any usages relying on suffix behavior (e.g., for "Contract"):
lazaroclapp
left a comment
lazaroclapp
left a comment
There was a problem hiding this comment.
Minor notes and questions, but LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java (3)
44-53: Update class Javadoc to reflect broader @contract recognition.We now recognize any simple-name Contract and custom ones via config, not only JetBrains.
Apply:
- * This Handler parses the jetbrains @Contract annotation and tries to check if the contract is + * This Handler parses @Contract-style annotations (JetBrains or any annotation with simple name + * "Contract", plus any configured custom annotations) and tries to check if the contract is
58-64: Make constraint sets static final (and name by role).Avoid per-instance allocation and clarify intent; introduce separate consequent set.
Apply:
- /** A set of value constraints in the antecedent which we can check for now. */ - private final Set<String> checkableValueConstraints = Set.of("_", "null", "!null"); - - /** All known valid value constraints */ - private final Set<String> allValidValueConstraints = - Set.of("_", "null", "!null", "true", "false"); + /** Antecedent constraints we can check now. */ + private static final Set<String> CHECKABLE_ANTECEDENT = Set.of("_", "null", "!null"); + /** All valid antecedent constraints (per JetBrains grammar). */ + private static final Set<String> VALID_ANTECEDENT = Set.of("_", "null", "!null", "true", "false"); + /** All valid consequent constraints. */ + private static final Set<String> VALID_CONSEQUENT = + Set.of("!null", "null", "true", "false", "fail", "new");
134-147: Guard against null return expressions (defensive).Unnecessary in practice for non-void methods, but cheap to guard.
Apply:
- Nullness nullness = + if (returnTree.getExpression() == null) { + return super.visitReturn(returnTree, null); + } + Nullness nullness = analysis .getNullnessAnalysis(returnState) .getNullnessForContractDataflow( new TreePath(returnState.getPath(), returnTree.getExpression()), returnState.context);nullaway/src/test/java/com/uber/nullaway/ContractsTests.java (1)
239-294: Nice: malformed custom and non‑JetBrains contracts. Consider one extra case.Add one case for an invalid consequent token (e.g., "!null -> notnul") to lock in consequent‑side validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java(1 hunks)nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java(3 hunks)nullaway/src/test/java/com/uber/nullaway/ContractsTests.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-14T18:50:06.159Z
Learnt from: msridhar
PR: uber/NullAway#1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.
Applied to files:
nullaway/src/test/java/com/uber/nullaway/ContractsTests.java
🧬 Code graph analysis (1)
nullaway/src/main/java/com/uber/nullaway/handlers/contract/ContractCheckHandler.java (1)
nullaway/src/main/java/com/uber/nullaway/ErrorMessage.java (1)
ErrorMessage(26-73)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build and test on ubuntu-latest
- GitHub Check: Build and test on macos-latest
- GitHub Check: Build and test on windows-latest
- GitHub Check: Build spring-framework with snapshot
- GitHub Check: Build caffeine with snapshot
🔇 Additional comments (4)
nullaway/src/main/java/com/uber/nullaway/handlers/Handlers.java (1)
81-81: Always adding ContractCheckHandler looks correct.This aligns with reporting malformed contracts regardless of CheckContracts, while body checks still honor the flag.
nullaway/src/test/java/com/uber/nullaway/ContractsTests.java (3)
21-45: Good: validates no body-check diagnostics without flag.Covers regression risk from always-installing ContractCheckHandler.
102-159: Good coverage for non‑JetBrains @contract by simple name; negative case included.This confirms recognition is by simple name and doesn’t overreach to NotContract.
197-237: Diagnostics anchored at declarations look right.Adds declaration‑site errors and excludes call‑site noise as intended.
Since version 0.12.11, NullAway accepts any annotation with simple name `Contract`. See uber/NullAway#1295.
Since version 0.12.11, NullAway accepts any annotation with `Contract` simple name. See uber/NullAway#1295.
Since version 0.12.11, NullAway accepts any annotation with `Contract` simple name. See uber/NullAway#1295.
Since version 0.12.11, NullAway accepts any annotation with `Contract` simple name. See uber/NullAway#1295. Signed-off-by: Ogu1208 <kdasunb6@gmail.com>
2 participants
Fixes #1222
Fixes #1215
We make the following changes to NullAway's handling of
@Contractannotations. First, NullAway now treats any annotation with simple nameContractas a contract annotation, not just the JetBrains@Contract. This improves the user experience when using third-party libraries containing non-Jetbrains@Contractannotations on public APIs, as those annotation types no longer need to be specified as a custom contract annotation to be picked up by NullAway. (The custom contract annotation option is still supported, in case someone wants to use an annotation as a contract that doesn't have simple nameContract.)Second, NullAway now only reports errors for malformed
@Contractannotations on the corresponding method declaration, not on calls to the method. Reporting malformed@Contracterrors on calls to methods was a bad behavior, since the method declaration could be in a library that cannot be easily modified.It's possible this change will lead to unexpected behavior, if users were writing non-Jetbrains annotations named
@Contractand with similar arguments to@Contractbut did not want them used by NullAway. But this seems unlikely, so we go ahead and make this change without an option to disable it. We can add such an option later if we hear about any issues.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation