apache / apache/datafusion-comet
`make format` output fails scalastyle IfBraceChecker (scalafmt wraps long brace-less if/else without adding braces)
- Dominant language
- Scala
- Stars
- 1.3k
- Forks
- 373
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 198
Description
### Describe the bug
`make format` does not produce code that passes the repo's own scalastyle check, and can itself introduce the violation.
`make format` runs `scalafix:scalafix` and `spotless:apply` (scalafmt). scalafmt (`scalafmt.conf`: `maxColumn = 98`, `rewrite.rules = [Imports]`) wraps a long brace-less `if`/`else` so that a branch spans multiple lines, but it never inserts braces — scalafmt has no brace-adding rewrite (`RedundantBraces` only removes them), and none of the configured scalafix rules (`ExplicitResultTypes, NoAutoTupling, RemoveUnused, DisableSyntax, LeakingImplicitClassVal, NoValInForComprehension, ProcedureSyntax, RedundantSyntax`) add them.
scalastyle's `IfBraceChecker` (`dev/scalastyle-config.xml`, `singleLineAllowed=true`, `doubleLineAllowed=true`) then fails the build with `If block needs braces`, because a brace-less branch that spans multiple lines is neither single- nor double-line. Since `make format` does not run scalastyle, it reports success while leaving code that the build/CI rejects, and re-running `make format` cannot fix it — it re-produces the same wrapped, brace-less form.
### Steps to reproduce
1. Write a brace-less `if`/`else` whose branch is a single expression longer than `maxColumn` (98) in a `.scala` file, e.g.:
```scala
def sourceId(field: Any, mirror: SomeReflection): Option[Int] =
if (dropped) None
else Some(mirror.getMethod(field.getClass, "sourceId").invoke(field).asInstanceOf[Int])
```
2. Run `make format`. scalafmt wraps the `else` across lines without adding braces:
```scala
if (dropped) None
else
Some(
mirror.getMethod(field.getClass, "sourceId")
.invoke(field)
.asInstanceOf[Int])
```
3. Build (e.g. `./mvnw test-compile -DskipTests`). scalastyle fails:
```
error file=.../MyFile.scala message=If block needs braces line=... column=...
```
4. Re-run `make format` — no change; the violation persists.
### Expected behavior
`make format` should produce code that passes the repository's own scalastyle checks. Formatting (`make format`) and linting (scalastyle in the build) should not disagree in a way that has no automated resolution.
### Additional context
There is currently no automated fixer for this in the toolchain: scalafmt (pinned `3.6.1`) cannot insert braces into control structures, and the enabled scalafix rules do not add them. Possible resolutions:
- Drop `IfBraceChecker` from `dev/scalastyle-config.xml`. scalafmt is the canonical formatter and owns brace/wrapping decisions, so this check overlaps with it and cannot be auto-satisfied for a long brace-less branch.
- Or, if enforcing braces is intended, document that a long `if`/`else` must be hand-braced (or its long branch hoisted into a `val` so the branch stays single-line), since `make format` will not do it.
Encountered when a long `else Some(...)` branch in `spark/src/main/scala/org/apache/comet/iceberg/IcebergReflection.scala` was wrapped by scalafmt and then rejected by scalastyle's `IfBraceChecker`.
Contributor guide
Assessment
This issue has not been assessed yet.