Repository navigation
Update to Scala 2.13, drop Scala 2.12 support - #557
Conversation
Up to standards ✅🟢 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 1 high |
| Comprehensibility | 2 minor |
🟢 Metrics 0 complexity · 4 duplication
Metric Results Complexity 0 Duplication 4
🟢 Coverage 98.13% diff coverage · +5.30% coverage variation
Metric Results Coverage variation ✅ +5.30% coverage variation (-0.50%) Diff coverage ✅ 98.13% diff coverage (50.00%) Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (bd57cda) Report Missing Report Missing Report Missing Head commit (1635ee5) 738 (-547) 641 (-407) 86.86% (+5.30%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#557) 107 105 98.13% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
Bumps scalaVersion/scalaBinaryVersion to 2.13, removes cross-building and 2.12-only compiler flags, and fixes the resulting 2.13 breakage: removed collection APIs (breakOut, .to[X], mapValues), parallel collections moved to a separate module, and guardrail bumped to 1.0.0-M1 (0.75.2's akka-http codegen emits invalid Scala under 2.13). case-app is bumped to 2.1.0 (no 2.13 build exists below that) and CLIApp's CLI parsing is rewritten off the removed case-app 1.x Command/CommandParser API onto the 2.x Parser/Help API, with Analyze's option case class split into an AdvancedOptions group to keep it under case-app's field-count ceiling. Known issue: case-app 2.1.0's Parser/Help derivation macro crashes scalac 2.13 (LambdaLift, "key not found: package command") whenever derivation for two or more distinct option types occurs in the same compiled package - reproduced in isolation, unrelated to code shape, and unaffected by bumping to 2.13.18. The cli module does not compile as a result; core, model, toolRepository-remote and codacy-api-client all build clean on 2.13.
…ashes Core: replace removed 2.13 collection APIs in tests (.to[X], breakOut). Cli: case-app's Parser/Help derivation macro crashes scalac 2.13 in two distinct, previously undiagnosed ways: - "key not found: package command" (LambdaLift) when the annotated case class is a bare top-level member of a package. - "assertion failed: static" (delambdafy) when Parser[X]/Help[X] are derived from a different object than the one X is declared in. Both fixed by nesting the option case classes in an Options object and declaring their derived Parser/Help vals right there, reused by the parsing objects instead of re-deriving. Also adds the missing CLIApp.parseCommand used by CLISpec since the case-app 2.x rewrite, and fixes tests still constructing Analyze with fields that moved into the nested AdvancedOptions group.
082b3e2 to
a0f5f92
Compare
There was a problem hiding this comment.
Pull Request Overview
The CLI does not compile under Scala 2.13 because the parser derivation still fails, so the migration is not usable as submitted. The top-level --help and -h flags also regress to an unknown-command error.
Several required build and CLI validation scenarios are missing, including successful compilation, removal of Scala 2.12 references, command parsing, error handling, and command execution.
Codacy also found:
- CLI complexity increased and four duplication findings were reported.
- CLI coverage is approximately 47%, including reduced changed-line coverage.
- An informational method-length finding was reported in
Tool.run.
Test suggestions
- Compile all project modules successfully with Scala 2.13.
- Verify the build no longer cross-builds or references Scala 2.12 artifacts.
- Exercise migrated collection conversions and map transformations in core, CLI, and toolRepository-remote.
- Parse valid analyze and validate-configuration command arguments.
- Handle CLI version, help, unknown commands, invalid arguments, and malformed duration/commit UUID values.
- Verify CLI analysis and configuration command execution after the parser migration.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Compile all project modules successfully with Scala 2.13.
2. Verify the build no longer cross-builds or references Scala 2.12 artifacts.
3. Parse valid analyze and validate-configuration command arguments.
4. Handle CLI version, help, unknown commands, invalid arguments, and malformed duration/commit UUID values.
5. Verify CLI analysis and configuration command execution after the parser migration.
Make this review better
TIP Add Codacy review.md instructions or an AGENTS.md so the reviewer knows your conventions.
TIP How was this review? Give us feedback
| val analyzeParser: Parser[Analyze] = Parser[Analyze] | ||
| val analyzeHelp: Help[Analyze] = Help[Analyze] | ||
|
|
||
| final case class ValidateConfiguration(@Recurse | ||
| options: CommonOptions, | ||
| @ExtraName("d") @ValueDescription( | ||
| "The directory where the configuration file is located") | ||
| directory: Option[File] = Option.empty) | ||
| extends CommandOptions | ||
| val validateConfigurationParser: Parser[ValidateConfiguration] = Parser[ValidateConfiguration] | ||
| val validateConfigurationHelp: Help[ValidateConfiguration] = Help[ValidateConfiguration] |
There was a problem hiding this comment.
🔴 HIGH RISK
The CLI still relies on parser derivation that fails under Scala 2.13, leaving this module uncompilable. Replace the failing parser dependency or provide a compiling implementation before considering the migration complete.
TIP React 👍 if this review is right, or reply with what's wrong and how it could be better to help improve the reviewer.
| terminate(ExitStatus.ExitCodes.success) | ||
| case "analyze" :: rest => AnalyzeParsing.parseAndRun(rest, run, terminate) | ||
| case "validate-configuration" :: rest => ValidateConfigurationParsing.parseAndRun(rest, run, terminate) | ||
| case unknown :: _ => |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Top-level --help and -h are rejected as unknown commands instead of displaying CLI help. Handle these flags before the unknown-command branch and add regression coverage.
TIP React 👍 if this review is right, or reply with what's wrong and how it could be better to help improve the reviewer.
Summary
scalaVersion/scalaBinaryVersionto 2.13, drops cross-building and 2.12-only compiler flags.core,toolRepository-remote, andcli(breakOut,.to[X],mapValues, parallel collections module).guardrailto1.0.0-M1(0.75.2's akka-http codegen emits invalid Scala under 2.13) and pins scalameta to keep the meta-build classpath consistent.case-appto2.1.0(no 2.13 build exists below that) and rewritesCLIApp's CLI parsing off the removed case-app 1.xCommand/CommandParserAPI onto the 2.xParser/HelpAPI. SplitsAnalyze's option case class into anAdvancedOptionsgroup to stay under case-app's field-count ceiling.Known issue - cli module does not compile
case-app 2.1.0's
Parser/Helpderivation macro crashes scalac 2.13 (LambdaLift,NoSuchElementException: key not found: package command) whenever derivation for two or more distinct option case classes happens in the same compiled package. Reproduced in isolation across many code shapes (nested/top-level, generic/non-generic, reduced field count, explicit/automatic derivation) - it is not fixable by restructuring this codebase. Also confirmed on Scala 2.13.18, so it isn't a scalac patch-version bug either.Since the CLI needs to parse both
analyzeandvalidate-configuration, this blocks theclimodule specifically.core,model,toolRepository-remote, andcodacy-api-clientall compile clean on 2.13.Opening this to see what CI reports; the
climodule build is expected to fail until case-app is replaced or the upstream bug is fixed.Test plan
climodule compile failure, other modules green)