scalameta / scalafmt

Code formatter for Scala
http://scalameta.org/scalafmt
Apache License 2.0
1.43k stars 277 forks source link

Version `3.8.3` formatting Scala code error (The maxColumn exceeds the limit) #4187

Closed panbingkun closed 1 month ago

panbingkun commented 2 months ago

Configuration (required)

align = none
align.openParenDefnSite = false
align.openParenCallSite = false
align.tokens = []
importSelectors = "singleLine"
optIn = {
  configStyleArguments = false
}
danglingParentheses.preset = false
docstrings.style = Asterisk
maxColumn = 98
runner.dialect = scala213
version = 3.8.3

Command-line parameters (required)

When I run scalafmt via CLI like this: scalafmt -c dev/.scalafmt.conf sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SessionHolder.scala

See the file here: https://github.com/apache/spark/blob/e1b5d65aeeccd2d724b753fc6f2132086825bef9/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SessionHolder.scala

Steps

Given code like this:

logWarning(
        log"Session plan cache is disabled due to non-positive cache size." +
          log" Current value of " +
          log"'${MDC(LogKeys.CONFIG, Connect.CONNECT_SESSION_PLAN_CACHE_SIZE.key)}' is ${MDC(
              LogKeys.CACHE_SIZE,
              SparkEnv.get.conf.get(Connect.CONNECT_SESSION_PLAN_CACHE_SIZE))}")

Problem

Scalafmt formats code like this:

logWarning(log"Session plan cache is disabled due to non-positive cache size." +
        log" Current value of " +
        log"'${MDC(LogKeys.CONFIG, Connect.CONNECT_SESSION_PLAN_CACHE_SIZE.key)}' is ${MDC(LogKeys.CACHE_SIZE, SparkEnv.get.conf.get(Connect.CONNECT_SESSION_PLAN_CACHE_SIZE))}")

Note: Obviously, the number of columns after formatting greatly exceeds the limit

Expectation

I would like the formatted output to look like this:

logWarning(
        log"Session plan cache is disabled due to non-positive cache size." +
          log" Current value of " +
          log"'${MDC(LogKeys.CONFIG, Connect.CONNECT_SESSION_PLAN_CACHE_SIZE.key)}' is ${MDC(
              LogKeys.CACHE_SIZE,
              SparkEnv.get.conf.get(Connect.CONNECT_SESSION_PLAN_CACHE_SIZE))}")

Workaround

Notes

the version 3.8.2 is normal, but the version 3.8.3 is abnormal.

panbingkun commented 2 months ago

When I was preparing to upgrade scalafmt from 3.8.2 to 3.8.3 in Spark repo, I found the above problem. The command is as follows:

./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl sql/connect/common -pl sql/connect/server -pl connector/connect/client/jvm
kitbellew commented 2 months ago

could you please edit your submission to make it more readable? not sure why you decided to retain all of the issue submission template, including text intended to describe what we're asking of you, and not the other way around.

kitbellew commented 2 months ago

When I was preparing to upgrade scalafmt from 3.8.2 to 3.8.3 in Spark repo, I found the above problem. The command is as follows:

./build/mvn scalafmt:format -Dscalafmt.skip=false -Dscalafmt.validateOnly=false -Dscalafmt.changedOnly=false -pl sql/connect/common -pl sql/connect/server -pl connector/connect/client/jvm

the original template, the one you kept in its entirety but didn't really follow, explains that you can't submit issues here unless you run the cli tool from this repository. if you use a maven plugin, feel free to report it to that plugin's maintainers instead.

panbingkun commented 2 months ago

Let me update it.

panbingkun commented 2 months ago
(base) ➜  spark-trunk git:(master) ✗ scalafmt --version
scalafmt 3.8.3
(base) ➜  spark-trunk git:(master) ✗ scalafmt -c dev/.scalafmt.conf sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SessionHolder.scala
(base) ➜  spark-trunk git:(master) ✗ git diff
image
panbingkun commented 2 months ago

Sorry, this is my first time submitting an issue to this repo, but it is indeed a issue. I have updated the description of this PR and it is normal in version 3.8.2. However, in the latest version 3.8.3, there is the aforementioned issue.