comphead opened a new issue, #5711:
URL: https://github.com/apache/datafusion-comet/issues/5711
### 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`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
