Fix lint violations in stdlib/test/ and stdlib/bootstrap-test/ (BT-964)#1005
Merged
Fix lint violations in stdlib/test/ and stdlib/bootstrap-test/ (BT-964)#1005
Conversation
… BT-964 - Remove 1974 unnecessary trailing `.` statement separators - Remove 662 unnecessary parentheses - Remove ~47 trailing `^` on last expressions (implicit returns) - Fix 5 non-self cascade candidates (arr/bus/c receivers) - Fix multi_expr_calc.bt fixture: replace effect-free non-last expressions - Lint: skip module-level expressions in effect_free_statement check (bootstrap-test files use module-level expressions as test assertions) - Lint: exempt `self` from cascade_candidate — consecutive sends to self (e.g. BUnit assertions) should remain as separate statements for readability and debuggability Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCascade and effect-free statement linters were changed to skip module-level/top-level expressions and to only inspect class (instance & class) and standalone method bodies; cascade checks now ignore runs of consecutive sends to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant LintRunner as Lint Runner
participant Pass as CascadeCandidatePass
participant AST as AST (Module/Class/Method)
participant Checker as CascadeChecker
LintRunner->>Pass: invoke check on module
Pass->>AST: iterate classes & standalone method defs
AST-->>Pass: method bodies (only)
Pass->>Checker: run check_sequence on each method body
alt run is 3+ sends to same receiver & receiver == self
Checker-->>Pass: skip (no lint)
else other potential cascade
Checker-->>Pass: emit lint finding
end
Pass-->>LintRunner: aggregated findings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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
🧹 Nitpick comments (2)
stdlib/test/fixtures/actor_collect_counter.bt (1)
15-16: Consider extracting a shared counter-increment helper.
self.count := self.count + 1is repeated across multiple methods/blocks. A tiny helper would reduce duplication and keep fixture intent clearer.Also applies to: 33-34, 48-54
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stdlib/test/fixtures/actor_collect_counter.bt` around lines 15 - 16, Extract the repeated increment logic into a small helper (e.g., increment_count or incSelfCount) that updates self.count (performing self.count := self.count + 1) and return/produce any needed value if callers expect it; then replace each direct occurrence of self.count := self.count + 1 (including the instance followed by item * 2 and other repeats) with calls to that helper to remove duplication and clarify intent, updating callers to use the helper's return value or side effect as appropriate.crates/beamtalk-core/src/lint/cascade_candidate.rs (1)
229-379: Add a regression test for skipped module-level sequences.Given the new scope behavior, a direct test for top-level
arr add:runs would lock this contract and prevent accidental reintroduction.💡 Suggested test addition
@@ #[test] + fn module_level_sequence_not_flagged() { + let diags = lint("arr add: 1\narr add: 2\narr add: 3\n"); + assert!( + diags.is_empty(), + "module-level sequences should be skipped, got: {diags:?}" + ); + } + + #[test] fn class_method_body_flagged() { let diags = lint( "Object subclass: Foo\n class\n setup: arr =>\n arr add: 1\n arr add: 2\n arr add: 3\n", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/beamtalk-core/src/lint/cascade_candidate.rs` around lines 229 - 379, Add a new unit test in cascade_candidate.rs that ensures top-level (module-level) consecutive sends are checked: create a #[test] (e.g. module_level_sequences_flagged) which calls the existing lint(...) helper with a snippet containing three consecutive top-level "arr add: X" sends (no enclosing class/method), assert that lint returns one diagnostic (Severity::Lint) and that the message mentions "cascade" (and optionally the run length). This locks the regression for skipped module-level sequences.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stdlib/test/fixtures/error_counter.bt`:
- Around line 1-2: Add the required license header to the top of this .bt source
file: insert a two-line header containing "Copyright 2026 James Casey" and
"SPDX-License-Identifier: Apache-2.0" before any existing content (e.g., before
the actor comment used by the tests/stdlib/exception_mutations.bt fixture) so
the file begins with the required license lines.
In `@stdlib/test/fixtures/nlr_baz.bt`:
- Around line 4-6: Update the stale fixture comments in NlrBaz to reflect the
current implementation: remove or reword the sentence "Do not simplify to `x +
x`" since the fixture now contains `x + x`, clarify that the test ensures
top-level ^ still works after NLR changes, and add the coding-guideline note
that ^ must be used only for early returns (not on the final expression) to
match the new return-style policy; ensure the comment references the top-level ^
and the `x + x` expression so readers can reconcile intent and code.
---
Nitpick comments:
In `@crates/beamtalk-core/src/lint/cascade_candidate.rs`:
- Around line 229-379: Add a new unit test in cascade_candidate.rs that ensures
top-level (module-level) consecutive sends are checked: create a #[test] (e.g.
module_level_sequences_flagged) which calls the existing lint(...) helper with a
snippet containing three consecutive top-level "arr add: X" sends (no enclosing
class/method), assert that lint returns one diagnostic (Severity::Lint) and that
the message mentions "cascade" (and optionally the run length). This locks the
regression for skipped module-level sequences.
In `@stdlib/test/fixtures/actor_collect_counter.bt`:
- Around line 15-16: Extract the repeated increment logic into a small helper
(e.g., increment_count or incSelfCount) that updates self.count (performing
self.count := self.count + 1) and return/produce any needed value if callers
expect it; then replace each direct occurrence of self.count := self.count + 1
(including the instance followed by item * 2 and other repeats) with calls to
that helper to remove duplication and clarify intent, updating callers to use
the helper's return value or side effect as appropriate.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (178)
crates/beamtalk-core/src/lint/cascade_candidate.rscrates/beamtalk-core/src/semantic_analysis/validators.rsstdlib/bootstrap-test/arithmetic.btstdlib/bootstrap-test/array_ops.btstdlib/bootstrap-test/equality.btstdlib/bootstrap-test/erlang_exceptions.btstdlib/bootstrap-test/errors.btstdlib/bootstrap-test/exceptions.btstdlib/bootstrap-test/symbol.btstdlib/test/abstract_classes_test.btstdlib/test/actor_class_local_var_test.btstdlib/test/actor_conditional_mutations_test.btstdlib/test/actor_coordination_test.btstdlib/test/actor_deadlock_test.btstdlib/test/actor_lifecycle_test.btstdlib/test/actor_local_mutations_test.btstdlib/test/actor_message_chaining_test.btstdlib/test/actor_non_local_return_test.btstdlib/test/actor_self_send_collect_test.btstdlib/test/actor_slot_test.btstdlib/test/architecture_doc_examples_test.btstdlib/test/arithmetic_operations_test.btstdlib/test/arithmetic_test.btstdlib/test/array_test.btstdlib/test/association_test.btstdlib/test/beamtalk_interface_test.btstdlib/test/block_evaluation_test.btstdlib/test/blocks_test.btstdlib/test/cast_send_test.btstdlib/test/character_test.btstdlib/test/class_builder_test.btstdlib/test/class_hierarchy_query_test.btstdlib/test/class_method_self_new_test.btstdlib/test/class_object_dispatch_test.btstdlib/test/class_reflection_test.btstdlib/test/class_self_dispatch_test.btstdlib/test/class_state_test.btstdlib/test/class_variables_singleton_test.btstdlib/test/class_variables_test.btstdlib/test/closures_advanced_test.btstdlib/test/codegen_doc_test.btstdlib/test/collection_mutations_test.btstdlib/test/collection_protocol_test.btstdlib/test/collections_test.btstdlib/test/compiled_method_test.btstdlib/test/counter_test.btstdlib/test/custom_collection_test.btstdlib/test/custom_exceptions_test.btstdlib/test/datetime_test.btstdlib/test/dictionary_test.btstdlib/test/display_string_test.btstdlib/test/doc_test_example.btstdlib/test/empty_method_self_test.btstdlib/test/erlang_interop_test.btstdlib/test/error_message_test.btstdlib/test/error_method_test.btstdlib/test/exception_mutations_test.btstdlib/test/field_mutations_do_test.btstdlib/test/file_io_test.btstdlib/test/file_stream_test.btstdlib/test/fixtures/abstract_shape.btstdlib/test/fixtures/actor_chaining_fixture.btstdlib/test/fixtures/actor_class_local_var.btstdlib/test/fixtures/actor_collect_counter.btstdlib/test/fixtures/actor_coordination_fixture.btstdlib/test/fixtures/actor_nlr_basic.btstdlib/test/fixtures/actor_nlr_computed.btstdlib/test/fixtures/actor_nlr_local_var.btstdlib/test/fixtures/actor_nlr_mutate.btstdlib/test/fixtures/actor_nlr_on_do.btstdlib/test/fixtures/actor_nlr_recursive.btstdlib/test/fixtures/actor_slot_fixture.btstdlib/test/fixtures/circle.btstdlib/test/fixtures/class_method_self_new.btstdlib/test/fixtures/class_state_actor.btstdlib/test/fixtures/conditional_mutation_counter.btstdlib/test/fixtures/counter.btstdlib/test/fixtures/deadlock_a.btstdlib/test/fixtures/deadlock_b.btstdlib/test/fixtures/error_counter.btstdlib/test/fixtures/error_message_helper.btstdlib/test/fixtures/event_bus_fixture.btstdlib/test/fixtures/field_mutation_accumulator.btstdlib/test/fixtures/field_mutation_counter.btstdlib/test/fixtures/field_mutation_nested.btstdlib/test/fixtures/hom_composability_actor.btstdlib/test/fixtures/local_mutation_actor.btstdlib/test/fixtures/logging_counter.btstdlib/test/fixtures/math_helper.btstdlib/test/fixtures/mixed_call_site_actor.btstdlib/test/fixtures/multi_expr_calc.btstdlib/test/fixtures/multi_keyword.btstdlib/test/fixtures/mutation_return_values.btstdlib/test/fixtures/nlr_bar.btstdlib/test/fixtures/nlr_baz.btstdlib/test/fixtures/nlr_doubler.btstdlib/test/fixtures/nlr_field_mutation.btstdlib/test/fixtures/nlr_foo.btstdlib/test/fixtures/nlr_nested_hom.btstdlib/test/fixtures/nlr_on_do.btstdlib/test/fixtures/nlr_return_test.btstdlib/test/fixtures/nlr_skipper.btstdlib/test/fixtures/non_literal_callable_actor.btstdlib/test/fixtures/number_list.btstdlib/test/fixtures/recursive_actor.btstdlib/test/fixtures/removal_test_child.btstdlib/test/fixtures/removal_test_parent.btstdlib/test/fixtures/sealed_counter.btstdlib/test/fixtures/self_ref_actor.btstdlib/test/fixtures/string_formatter.btstdlib/test/fixtures/tier2_block_test_actor.btstdlib/test/fixtures/typed_account.btstdlib/test/fixtures/typed_counter.btstdlib/test/fixtures/typed_ledger.btstdlib/test/fixtures/untyped_container.btstdlib/test/fixtures/value_point.btstdlib/test/fixtures/value_type_builder.btstdlib/test/future_auto_await_test.btstdlib/test/hom_composability_test.btstdlib/test/instance_class_var_access_test.btstdlib/test/instance_class_var_access_value_type_test.btstdlib/test/integer_bitwise_test.btstdlib/test/json_test.btstdlib/test/keyword_messages_test.btstdlib/test/language_features_doc_test.btstdlib/test/list_literals_test.btstdlib/test/math_test.btstdlib/test/metaclass_test.btstdlib/test/method_resolver_hierarchy_test.btstdlib/test/mixed_call_site_test.btstdlib/test/mutation_return_values_test.btstdlib/test/nested_to_do_test.btstdlib/test/nil_protocol_test.btstdlib/test/nlr_state_threading_test.btstdlib/test/non_literal_callable_test.btstdlib/test/number_methods_test.btstdlib/test/object_protocol_test.btstdlib/test/pattern_matching_test.btstdlib/test/protoobject_actors_test.btstdlib/test/protoobject_manual_test.btstdlib/test/random_test.btstdlib/test/recursion_test.btstdlib/test/reflection_basic_test.btstdlib/test/regex_test.btstdlib/test/remove_from_system_test.btstdlib/test/runtime_doc_test.btstdlib/test/sealed_class_test.btstdlib/test/self_reference_test.btstdlib/test/semantic_scope_test.btstdlib/test/set_test.btstdlib/test/setup_state_test.btstdlib/test/source_file_reload_test.btstdlib/test/species_test.btstdlib/test/stack_frame_test.btstdlib/test/stream_collections_test.btstdlib/test/stream_test.btstdlib/test/string_advanced_methods_test.btstdlib/test/string_interpolation_test.btstdlib/test/string_new_methods_test.btstdlib/test/string_operations_test.btstdlib/test/string_quote_escape_test.btstdlib/test/system_test.btstdlib/test/test_case_assertion_test.btstdlib/test/test_runner_test.btstdlib/test/tier2_block_round_trip_test.btstdlib/test/times_repeat_simple_test.btstdlib/test/tuple_test.btstdlib/test/typed_classes_test.btstdlib/test/unary_messages_test.btstdlib/test/unicode_test.btstdlib/test/value_object_self_send_test.btstdlib/test/value_object_test.btstdlib/test/value_subclass_test.btstdlib/test/value_type_local_vars_test.btstdlib/test/value_type_multi_expr_test.btstdlib/test/value_type_non_local_return_test.btstdlib/test/value_type_update_test.btstdlib/test/workspace_interface_test.bt
- error_counter.bt: add missing license header - nlr_baz.bt: remove stale comment that contradicted implementation (the ^ was removed by the lint fix; comment about 'do not simplify to x + x' no longer applies since the body is now x + x per project rules) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.statement separators^on last expressions (use^only for early returns)selfcascade candidates (arr/bus/creceivers) to cascade syntaxmulti_expr_calc.btfixture: replaces effect-free non-last expressions with unary sendsLint changes
Two targeted lint improvements to eliminate false positives:
effect_free_statement: Skip module-level expression sequences. Bootstrap-test files use module-level expressions as test assertions (paired with// =>comments) — these are intentionally evaluated and should not be flagged.cascade_candidate: Exemptselffrom cascade checking. Consecutive sends toself(e.g. BUnit'sself assert:…; assert:…) would be harder to debug as a cascade — independent test assertions should remain as separate statements so failures can be isolated.Test plan
beamtalk lint stdlib/test/→ 0 violationsbeamtalk lint stdlib/bootstrap-test/→ 0 violationsjust test-stdlib→ 237 tests passed, 645 BUnit tests passedjust test→ all unit tests passedjust clippy→ passedhttps://linear.app/beamtalk/issue/BT-964
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Style