feat(rules): add Scala-specific review rules - #1095
Open
Qiyuanqiii wants to merge 1 commit into
Open
Conversation
Contributor
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). |
13 tasks
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
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.
Description
Summary
.scalareview inputs toscala.mdinstead of the generic default rules.Existing Gap and Fix
.scalais already present ininternal/config/allowlist/supported_file_types.json, so Scala files enter review. However,system_rules.jsonhad no Scala mapping and the embedded rule documents had noscala.md. A Scala source file without a higher-priority user rule therefore fell back todefault.md.This change adds:
The resolver regression case uses:
and verifies that the resolved rule contains
Scala Review Principles, a stable heading absent fromdefault.md. Removing the mapping or rule document, or falling back to the generic rule, makes the test fail.Manual verification now reports:
Rule Coverage
Some(null), unsafeOption.get,.nn, andunsafeNullsmapValues, infiniteLazyList/Stream, and proven quadratic workFuture.apply, and racing promisesthis, lazy cycles, and retried lazy side effectsequals/hashCodelaws, mutable keys, array content equality,Ordering, narrowing, overflow, NaN, and range endpointsNonFatal,Tryextraction, owned-resource closure, lazy values escaping resource scopes, and cleanup semanticsPrecision and False-Positive Controls
The rule document deliberately avoids becoming a Scala style guide.
scalaVersion,crossScalaVersions, compiler options, and the target platform before making version- or runtime-dependent claims.Futuresemantics from Cats Effect, ZIO, Akka, Pekko, Spark, Play, or framework-owned abstractions.The final wording was checked against primary Scala documentation. That review tightened several cross-version and cross-platform boundaries, including maps with explicit defaults,
Future.applyversus arbitrary futures, Scala 2 refutable-pattern failures,runtimeChecked,Usingsuppression semantics, Java collection adapters, and correctly declared Scala.js facades.Compatibility and Non-Goals
.scalawas already allowlisted..sbtor.scsupport.Type of Change
How Has This Been Tested?
Validation Environment
go1.26.5 windows/amd64eda25482c74dff2f03bed2648497f23c512effe9(origin/main)f44e27bGNU Make was unavailable on the validation host, so each constituent Makefile command was executed directly.
Go and Repository Validation
make testequivalent passed for every package returned bygo list ./..., excluding/extensions/:go test -v -race -count=1 ...internal/config/rulescoverage was 91.7%go mod tidy,gofmt -s -w ., andgo vetacross the Makefile package set completed successfullyscripts/add-license.shreported zero headers to add because the new file is Markdowngit diff --check, the staged whitespace check, and a separate no-index check for the new file passedocr rules check src/main/scala/com/example/Service.scalareported the system pattern**/*.scalaand the Scala ruleJavaScript and Pages Validation
The complete root JavaScript GitHub Actions test sequence did not pass on this Windows host because the action-contract harness received
status=nullfrom its spawned shell. Its translation-sync test passed independently.Installation under
pages/was attempted, butnpm installmade no progress for an extended period and was terminated. Consequently, the Pageslint,test,typecheck, andbuildscripts were not run locally. The pull-request Pages workflow subsequently completed successfully, covering lint, tests, type checking, and the production build.Current-Head GitHub Actions
All checks on commit
f44e27bpassed:OCR Self-Review
The repository-required OCR review completed successfully:
OCR selected only
system_rules.json. The six Markdown files were excluded asunsupported_ext, andsystem_rules_test.gowas excluded by the default test-path filter. This result is therefore not presented as full eight-file OCR coverage; the Scala document received a separate line-by-line factual review against primary Scala sources.Checklist
go fmt,go vet)Known Limitations
**/*.{kt,kts}and**/*.scalaplus both regression cases.Related Issues
Closes #1094.