improvement: Generate braceless Scala 3 templates in NewFileProvider - #8542
improvement: Generate braceless Scala 3 templates in NewFileProvider#8542jozanek wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds configurable Scala 3 braceless syntax generation for new files. ChangesBraceless New File Generation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client as LSP Client
participant MetalsLspService
participant NewFileProvider
participant BracelessSyntax
participant PackageProvider
Client->>MetalsLspService: Create new Scala file
MetalsLspService->>NewFileProvider: create file with userConfig supplier
NewFileProvider->>BracelessSyntax: inspect nearby source style
BracelessSyntax-->>NewFileProvider: style preference
NewFileProvider->>NewFileProvider: apply policy and compiler options
NewFileProvider->>PackageProvider: packageStatement(braceless)
PackageProvider-->>NewFileProvider: package-object text
NewFileProvider-->>MetalsLspService: generated file content
MetalsLspService-->>Client: created file
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| |inlay-hints.closing-labels.enable boolean false Should display closing label hints for methods/classes/objects next to their closing braces | ||
| |enable-semantic-highlighting boolean true Use semantic tokens highlight | ||
| |enable-indent-on-paste boolean false Indent snippets when pasted. | ||
| |use-braceless-syntax-for-new-files boolean false Use braceless syntax for newly generated Scala 3 files |
There was a problem hiding this comment.
I wonder if we could avoid the setting altogether, since it might be hard to discover for users. We could, potentially, check first scala file that exists already within the source root. Then we can see if the tree there is using braces within any topelevel symbols. If no other files are present the default could be handled by -no-indent or -indent flag flag set by the user and then finally use braces for the final default.
What do you think?
There was a problem hiding this comment.
Good idea, done. Dropped the setting and infer the style:
- Existing sources: a token detector checks the nearest sources (target package up to the source root), then falls back to the
-indentscalac option, then braces. - Compiler:
-no-indent/-old-syntax/migrationmode always force braces. - Override: kept a light
new-files-braceless-syntax: auto | always | never(defaultauto) so empty or mixed projects can opt in or force a choice.
Braceful projects are unchanged. ImplementAbstractMembers is untouched since its Scala 3 generation is upstream in dotty.
…lways|never override
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/src/test/scala/tests/NewFileLspSuite.scala (1)
571-589: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest name doesn't match what it verifies.
"scala2-always-braces"never passesbracelessSyntax = Some("always"); it only exercises the defaultAutopolicy on Scala 2 next to a braceless-looking sibling. The actual "alwaysoverride + existing sibling on Scala 2" combination (implied by the name) is untested — the closest existing test (braceless-syntax-always-ignored-on-scala2, lines 557-569) has no sibling files.✏️ Suggested fix: either rename or add the missing parameter
- // Scala 2 never uses braceless, even next to a braceless-looking sibling. - checkScala("scala2-always-braces")( + // Scala 2 never uses braceless, even with `always` set and a braceless-looking sibling. + checkScala("scala2-always-braces")( directory = Some("a/src/main/scala/foo/"), fileType = Right(Class), fileName = Right("Foo"), expectedFilePath = "a/src/main/scala/foo/Foo.scala", expectedContent = s"""|package foo | |class Foo { |$indent |} |""".stripMargin, existingFiles = """|/a/src/main/scala/foo/Existing.scala |package foo | |object Existing: | def value = 1 |""".stripMargin, + bracelessSyntax = Some("always"), )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/src/test/scala/tests/NewFileLspSuite.scala` around lines 571 - 589, Rename the test currently labeled "scala2-always-braces" to reflect that it verifies Scala 2’s default Auto policy beside a braceless-looking sibling, or pass bracelessSyntax = Some("always") to make the name accurate. Prefer also adding a separate test covering the explicit always override with an existing sibling, complementing "braceless-syntax-always-ignored-on-scala2".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/unit/src/test/scala/tests/NewFileLspSuite.scala`:
- Around line 571-589: Rename the test currently labeled "scala2-always-braces"
to reflect that it verifies Scala 2’s default Auto policy beside a
braceless-looking sibling, or pass bracelessSyntax = Some("always") to make the
name accurate. Prefer also adding a separate test covering the explicit always
override with an existing sibling, complementing
"braceless-syntax-always-ignored-on-scala2".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d748c773-7a48-4352-8060-c5584e7ba0cf
📒 Files selected for processing (7)
metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scalametals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scalametals/src/main/scala/scala/meta/internal/metals/newScalaFile/BracelessSyntax.scalametals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scalatests/unit/src/test/scala/tests/BracelessSyntaxSuite.scalatests/unit/src/test/scala/tests/NewFileLspSuite.scalatests/unit/src/test/scala/tests/UserConfigurationSuite.scala
🚧 Files skipped from review as they are similar to previous changes (2)
- metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
- metals/src/main/scala/scala/meta/internal/metals/newScalaFile/NewFileProvider.scala
2 participants
Implements the
NewFileProviderhalf of #3243: the New Scala File command (including the Package Object kind) can now emit brace-free Scala 3 stubs.Rather than a boolean opt-in, the style is inferred automatically so it works with zero configuration and matches the surrounding project:
class Foo {…}class Foopackage object foo {…}package object fooenum Color { case }enum Color:/caseHow the style is chosen (
new-files-braceless-syntaxsetting)Only Scala 3 is ever braceless. Beyond that:
-no-indent,-old-syntax, or a-source:*-migrationmode), braces are always used, overriding everything below.new-files-braceless-syntax(defaultauto):auto— match the style of nearby existing sources: the target package directory and its enclosing directories up to the source root, inspecting each directory's.scalafiles in a stable, filename-sorted order and stopping at the first top-level definition with a determinate style. Falls back to the-indentscalac option, then braces.always— always braceless (Scala 3).never— always braces (fully backward-compatible).Braces remain the outcome for braceful projects, so existing users see no change unless their project is already braceless.
Bodyless empty stubs
Empty class/trait/object/package-object stubs are generated bodyless (
class Foo, notclass Foo:+ empty body, which is a parse error in Scala 3); a bodyless declaration compiles and the cursor lands after the name.enumkeeps the existing incomplete-casetemplate (a bodylessenumdoesn't compile either).Scope (intentionally narrowed — does not close #3243)
The issue also names
ImplementAbstractMembers. That is not included here: its Scala 3 stub generation lives in the upstream dotty presentation compiler (shipped viascala3-compiler), and it already respects/produces braceless style for existing files. Making "Implement all members" braceless from scratch would require an upstream dotty change (and aPresentationCompilerConfigflag it consumes), not a Metals-only change. Leaving #3243 open to track that half.Summary by CodeRabbit
new-files-braceless-syntaxsetting (auto,always,never), including automatic style inference whenauto.