Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
panbingkun opened this issue Aug 27, 2024 · 6 comments · Fixed by #4254
Closed

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

panbingkun opened this issue Aug 27, 2024 · 6 comments · Fixed by #4254

Comments

@panbingkun
Copy link

panbingkun commented Aug 27, 2024

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Author

Let me update it.

@panbingkun
Copy link
Author

panbingkun commented Aug 27, 2024

(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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants