apache / apache/datafusion-comet

`make format` output fails scalastyle IfBraceChecker (scalafmt wraps long brace-less if/else without adding braces)

Open
#5,711 4 comments 0 reactions 1 assignee Claimed by @AshishGhodvinde View on GitHub
area:ci good first issue
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.