Use isEmpty() in preference to size()
#698
Conversation
|
Warning Rate limit exceeded@mernst has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR replaces explicit collection size comparisons (e.g., Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
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 |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
java/daikon/dcomp/DCRuntime.java (1)
2232-2240: Consider updating line 2235 for consistency.Line 2233 was updated to use
isEmpty(), but line 2235 still usessize() == 0. For consistency with the broader refactoring in this PR, consider updating line 2235 as well:@Pure @Override public int compareTo(@GuardSatisfied DVSet this, DVSet s1) { if (s1.isEmpty()) { return 1; - } else if (size() == 0) { + } else if (isEmpty()) { return -1; } else { return this.get(0).compareTo(s1.get(0)); } }java/daikon/inv/Equality.java (1)
106-114: Move emptiness assert before leader() to avoid premature NoSuchElementException.Currently leader() runs before asserting non-empty input. Reordering improves failure mode without behavior change.
Apply:
- VarInfo leader = leader(); - - // ensure well-formedness and set equality slots - assert !variables.isEmpty(); + // ensure well-formedness and set equality slots + assert !variables.isEmpty(); + VarInfo leader = leader();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (45)
java/daikon/AnnotateNullable.java(1 hunks)java/daikon/Daikon.java(6 hunks)java/daikon/DaikonSimple.java(2 hunks)java/daikon/Debug.java(1 hunks)java/daikon/DynamicConstants.java(5 hunks)java/daikon/FileIO.java(1 hunks)java/daikon/MergeInvariants.java(5 hunks)java/daikon/PptMap.java(1 hunks)java/daikon/PptRelation.java(5 hunks)java/daikon/PptSlice.java(3 hunks)java/daikon/PptSliceEquality.java(8 hunks)java/daikon/PptTopLevel.java(16 hunks)java/daikon/PrintInvariants.java(4 hunks)java/daikon/SplitDtrace.java(2 hunks)java/daikon/UnionInvariants.java(1 hunks)java/daikon/ValueTuple.java(1 hunks)java/daikon/VarInfoName.java(3 hunks)java/daikon/chicory/Instrument.java(1 hunks)java/daikon/chicory/Instrument24.java(1 hunks)java/daikon/chicory/Runtime.java(2 hunks)java/daikon/dcomp/DCRuntime.java(1 hunks)java/daikon/derive/unary/SequenceMinMaxSumFactory.java(1 hunks)java/daikon/diff/MatchCountVisitor.java(2 hunks)java/daikon/diff/MultiDiffVisitor.java(2 hunks)java/daikon/inv/Equality.java(2 hunks)java/daikon/inv/filter/InvariantFilters.java(1 hunks)java/daikon/inv/filter/ParentFilter.java(1 hunks)java/daikon/inv/unary/scalar/NonModulus.java(1 hunks)java/daikon/simplify/LemmaStack.java(1 hunks)java/daikon/split/FileCompiler.java(2 hunks)java/daikon/split/PptSplitter.java(3 hunks)java/daikon/split/SpinfoFile.java(1 hunks)java/daikon/split/SplitterFactory.java(1 hunks)java/daikon/split/SplitterList.java(2 hunks)java/daikon/suppress/NISuppressionSet.java(1 hunks)java/daikon/test/Common.java(1 hunks)java/daikon/test/FormatTestCase.java(1 hunks)java/daikon/test/SampleTester.java(2 hunks)java/daikon/test/VarInfoNameDriver.java(1 hunks)java/daikon/tools/ExtractConsequent.java(2 hunks)java/daikon/tools/InvariantChecker.java(2 hunks)java/daikon/tools/compare/LogicalCompare.java(2 hunks)java/daikon/tools/jtb/CreateSpinfo.java(1 hunks)java/daikon/tools/runtimechecker/InstrumentVisitor.java(6 hunks)java/daikon/tools/runtimechecker/WriteViolationFile.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
java/daikon/PptTopLevel.java (1)
java/daikon/suppress/NIS.java (1)
NIS(46-1178)
⏰ 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). (8)
- GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
- GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
- GitHub Check: build
🔇 Additional comments (53)
java/daikon/test/FormatTestCase.java (1)
907-907: LGTM!The refactoring from
samples.size() == 0tosamples.isEmpty()is correct and follows Java best practices. The isEmpty() method is more idiomatic and clearly expresses the intent of checking for an empty collection.java/daikon/test/VarInfoNameDriver.java (1)
68-68: LGTM!The change to
list.isEmpty()correctly modernizes the blank-line check. This is more readable and idiomatic than comparing size to zero.java/daikon/derive/unary/SequenceMinMaxSumFactory.java (1)
44-44: LGTM!The change to
result.isEmpty()is correct and consistent with the PR's objective to use idiomatic emptiness checks. This clearly expresses the intent to return null when no derivations were enabled.java/daikon/chicory/Instrument24.java (1)
183-183: LGTM!The change to
!Runtime.ppt_select_pattern.isEmpty()correctly modernizes the check for whether any ppt_select patterns were specified. This is more idiomatic than comparing size to zero.java/daikon/SplitDtrace.java (1)
59-59: LGTM!Both changes to
rec.isEmpty()are correct. These termination checks for record-reading loops are more idiomatic with isEmpty() than size() comparisons.Also applies to: 102-102
java/daikon/simplify/LemmaStack.java (1)
428-428: LGTM!The change to
problems.isEmpty()is correct and improves readability. This clearly expresses the check for whether any problematic lemmas were identified during minimization.java/daikon/test/Common.java (1)
63-63: LGTM!The change to
!vlist.isEmpty()correctly modernizes the check for whether any enclosing variables were found. This is more idiomatic than comparing size to zero.java/daikon/tools/jtb/CreateSpinfo.java (1)
260-260: LGTM!The change to
!method_conds.isEmpty()is correct. This more clearly expresses the intent to only emit PPT_NAME entries when conditions exist for a method.java/daikon/ValueTuple.java (1)
432-440: LGTM!The
isEmpty()method implementation is correct and follows standard Java conventions. The method is properly annotated with@Pureand includes clear JavaDoc documentation.java/daikon/PptMap.java (1)
249-257: LGTM!The
isEmpty()method implementation is correct and consistent with standard Java conventions. Proper annotations and documentation are in place.java/daikon/AnnotateNullable.java (1)
174-174: LGTM!The change from
size() == 0toisEmpty()is more idiomatic and improves code readability while preserving the same behavior.java/daikon/inv/filter/ParentFilter.java (1)
155-155: LGTM!The refactoring from
size() > 0to!isEmpty()is correct and more idiomatic. The behavior is equivalent and readability is improved.java/daikon/split/SpinfoFile.java (1)
235-235: LGTM!The change to use
isEmpty()is correct and more idiomatic for checking if aListis empty. The behavior is preserved.java/daikon/suppress/NISuppressionSet.java (1)
503-503: LGTM!The update to the commented-out assertion maintains consistency with the broader refactoring effort. Even though the assertion is disabled (as explained by the comment), keeping it up-to-date is good practice.
java/daikon/FileIO.java (1)
1756-1763: Good swap to isEmpty() + clearer error message.Semantics unchanged and diagnostics improved by including the filename.
java/daikon/diff/MultiDiffVisitor.java (1)
152-153: Use of isEmpty() is correct and clearer.No behavioral change; collections are non-null in both sites.
Also applies to: 215-216
java/daikon/inv/filter/InvariantFilters.java (1)
101-126: LGTM: idiomatic emptiness check.
!variableFilters.isEmpty()reads better; no behavior change.java/daikon/chicory/Instrument.java (1)
146-153: LGTM: clearer inclusion guard with isEmpty().Semantics unchanged; logging paths preserved.
java/daikon/PptSlice.java (1)
146-151: LGTM: isEmpty() in invariant-removal paths.No semantic change; improves readability in hot paths.
Also applies to: 166-171, 270-273
java/daikon/chicory/Runtime.java (1)
433-435: LGTM: idiomatic emptiness checks.
isEmpty()reads better and avoids computing size; behavior unchanged.Also applies to: 682-690
java/daikon/test/SampleTester.java (1)
593-595: LGTM: argument validation uses isEmpty().Clarity improved; same failure modes.
Also applies to: 632-634
java/daikon/UnionInvariants.java (1)
153-155: Verify if null-safety guard aligns with project conventions.The review suggests adding a null check for
joiner_viewbased on a preexisting risk. However:
joiner_viewis initialized at field declaration (PptTopLevel.java:395) and always reassigned to new instances; no evidence of null assignment exists in the codebase.- No existing null checks on
joiner_viewappear elsewhere in the codebase.- The actual change (replacing
size()==0withisEmpty()) is unrelated to this null-safety concern.While the suggested guard is defensively sound, verify whether your project requires such defensive null checks for public fields, or if the current initialization guarantees are sufficient.
java/daikon/tools/InvariantChecker.java (1)
242-242: LGTM! Idiomatic emptiness checks.The replacements of
size() == 0withisEmpty()are semantically equivalent and follow Java best practices for collection emptiness checks.Also applies to: 252-252
java/daikon/inv/unary/scalar/NonModulus.java (1)
146-146: LGTM! Idiomatic emptiness check.The replacement of
elements.size() == 0withisEmpty()is semantically equivalent and follows Java best practices.java/daikon/diff/MatchCountVisitor.java (1)
163-163: LGTM! Idiomatic emptiness checks.The replacements of
size() == 0withisEmpty()in bothcalcRecall()andcalcPrecision()are semantically equivalent and follow Java best practices.Also applies to: 198-198
java/daikon/tools/runtimechecker/WriteViolationFile.java (1)
112-112: LGTM! Idiomatic emptiness check.The replacement of
vios.size() == 0withisEmpty()is semantically equivalent and follows Java best practices.java/daikon/split/SplitterList.java (1)
181-181: LGTM! Idiomatic emptiness check.The replacement of
splitterArrays.size() == 0withisEmpty()is semantically equivalent and follows Java best practices.java/daikon/split/SplitterFactory.java (1)
133-133: LGTM! Idiomatic emptiness check.The replacement of
sp.size() >= 1with!sp.isEmpty()is semantically equivalent and more clearly expresses the intent to check for a non-empty list.java/daikon/PrintInvariants.java (4)
487-487: LGTM! Idiomatic emptiness check.The replacement of
my_ppt.parents.size() == 0withisEmpty()is semantically equivalent and follows Java best practices.
883-883: LGTM! Idiomatic emptiness check.The replacement in the compound condition using
ppt.joiner_view.invs.isEmpty()is semantically equivalent and more readable.
1015-1015: LGTM! Idiomatic emptiness checks.The replacements of
size() > 0with!isEmpty()for modified_vars, reassigned_parameters, and unmodified_vars are semantically equivalent and follow Java best practices.Also applies to: 1022-1022, 1030-1030
1050-1050: LGTM! Idiomatic emptiness check.The replacement of
mods.size() > 0with!mods.isEmpty()is semantically equivalent and follows Java best practices.java/daikon/split/FileCompiler.java (1)
153-153: LGTM! Idiomatic emptiness checks.The replacements of
filenames.size() == 0withisEmpty()andretry.size() > 0with!retry.isEmpty()are semantically equivalent and follow Java best practices.Also applies to: 253-253
java/daikon/DaikonSimple.java (2)
111-113: Idiomatic emptiness check.Using isEmpty() improves clarity; logic unchanged.
597-599: ValueTuple.isEmpty() API confirmed and semantically correct.The isEmpty() method exists in ValueTuple (lines 438–439) and correctly returns true when the ValueTuple has zero trace values. Its implementation—
return size() == 0;—delegates to the size() method (lines 424–430), which returnsvals.length. The assertion at line 597 of DaikonSimple.java is valid: when a program point has no variables (line 592), the ValueTuple should be empty. No issues found.java/daikon/Debug.java (1)
193-201: LGTM: simpler non-empty branch.!isEmpty() is the right idiom here; behavior preserved.
java/daikon/tools/ExtractConsequent.java (1)
236-244: LGTM: output gated by set emptiness.!isEmpty() reads better; logic unchanged.
java/daikon/inv/Equality.java (1)
257-262: LGTM: isEmpty() in leader selection.Clearer than size() checks; semantics preserved.
java/daikon/PptSliceEquality.java (1)
271-307: LGTM: consistent isEmpty() usage and explicit non-empty asserts.These changes improve readability without altering behavior. Assertions correctly document pre/postconditions.
Also applies to: 338-346, 366-372, 392-395, 408-414, 508-513, 517-525, 596-602
java/daikon/tools/runtimechecker/InstrumentVisitor.java (1)
940-947: LGTM: parameter list separators use !isEmpty().Concise and correct for codegen; no behavior change.
Also applies to: 978-985, 1048-1054, 1214-1218, 1228-1232, 1272-1276
java/daikon/Daikon.java (6)
742-747: LGTM: input-files emptiness check.Clearer error path; no behavior change.
1584-1590: LGTM: decl_files emptiness check before progress message.Improves clarity.
2133-2142: LGTM: map_files guarded by both flags and emptiness.Keeps I/O quiet when unused.
1814-1819: LGTM: parents.isEmpty() for root detection.Equivalent, more idiomatic.
2767-2781: LGTM: !vars.isEmpty() guard in undoOpts.No functional change; clearer.
2363-2375: No issues found — PptMap.isEmpty() exists.The
isEmpty()method is available in PptMap at line 255 of java/daikon/PptMap.java as a public method annotated with @pure. The code change usingall_ppts.isEmpty()is valid.java/daikon/DynamicConstants.java (1)
520-526: Good swap to Collection.isEmpty()Semantics preserved and readability improved across all checks. No issues.
Also applies to: 531-534, 702-706, 1099-1102, 1127-1129
java/daikon/tools/compare/LogicalCompare.java (1)
435-446: Loop/guard now idiomatic with isEmpty()Equivalent control flow with subList. Looks good.
Also applies to: 447-451
java/daikon/PptRelation.java (1)
229-229: isEmpty() usage reads betterAll parents/children emptiness checks are correct and clearer.
Also applies to: 872-875, 1032-1036, 1043-1046
java/daikon/VarInfoName.java (1)
3438-3459: isEmpty() refactor is fineNo semantic change; remains consistent with surrounding size() checks.
Also applies to: 3465-3498, 3561-3599
java/daikon/split/PptSplitter.java (1)
433-437: Adopting isEmpty() across implication builderSemantics preserved; cleaner checks on invariants and exclusives.
Also applies to: 440-441, 478-503, 506-510
java/daikon/PptTopLevel.java (1)
353-353: LGTM: Idiomatic isEmpty() refactoring applied consistently.All changes replace size-based emptiness checks with the idiomatic
isEmpty()method. The transformations are semantically equivalent and follow the correct pattern:
size() == 0→isEmpty()size() > 0→!isEmpty()size() != 0→!isEmpty()The change at Line 3638 (
!rel.isEmpty()) correctly uses the newisEmpty()method added toPptRelationas part of this PR.Also applies to: 1025-1025, 1042-1042, 1181-1181, 1216-1216, 1224-1224, 1566-1566, 3137-3137, 3595-3596, 3638-3638, 3870-3870, 3926-3926, 3959-3959, 3993-3993, 4113-4113, 4266-4266, 4355-4355
java/daikon/MergeInvariants.java (1)
228-228: LGTM: Consistent isEmpty() usage across PPT hierarchy checks.All changes correctly replace size comparisons with
isEmpty()calls for checking collection emptiness. The refactoring maintains the original semantics while improving code readability:
- Line 228: Splitter files presence check
- Lines 291, 295, 359, 364: Assertions ensuring PPT children exist
- Line 307: Assertion ensuring no children in splitter PPTs
- Line 377: Checking for root PPTs (no parents)
Also applies to: 291-291, 295-295, 307-307, 359-359, 364-364, 377-377
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
No description provided.