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

scalafmtCheck not accepting formatted code #1231

Closed
fdietze opened this issue Jun 29, 2018 · 10 comments
Closed

scalafmtCheck not accepting formatted code #1231

fdietze opened this issue Jun 29, 2018 · 10 comments

Comments

@fdietze
Copy link

fdietze commented Jun 29, 2018

addSbtPlugin("com.geirsson" % "sbt-scalafmt" % "1.6.0-RC3")

(default configuration)

In my sbt project I have several (heavily reduced) scala-files. When I run scalafmt and then scalafmtCheck, These files get reported as not being properly formatted. The second one only gets reported after the first one is changed to be properly formatted.

object MainViewParts {

  val syncModeSwitcher =
    span(
      state.syncMode
        .map {
          mode =>
            span(
              title := "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
              onClick
                .map(
                  _ =>
                    (if (mode == SyncMode.Live) SyncMode.Local
                     else SyncMode.Live): SyncMode
                ) --> state.syncMode
            )
        }
    )

}

Shortening the string makes this one work.

object TestView extends View {
  override def apply() = {
    div(
      div(
        (5 to 5).map { res => 5},
        //
      )
    )
  }
}

Removing the comment makes this one work.

object ViewConfigParser {
  val page: P[Page] = P( pageMode ~/ (pageSeparator ~/ nodeIdList ~ (pageSeparator ~ nodeIdList).?).? ~/ (urlSeparator | End) )
    .map {
      case x => Page()
    }
}

Do you need more examples?

@olafurpg
Copy link
Member

olafurpg commented Jul 2, 2018

Thanks for reporting! I am unable to reproduce, I tried both with the cli and sbt plugin.

Can you please update https://github.com/olafurpg/repro-scalafmt/blob/master/src/main/scala/Issue1231.scala with an input that reproduces the issue?

@fdietze
Copy link
Author

fdietze commented Jul 2, 2018

This was hard to find out: running scalafmt a second time in the same sbt session doesn't do anything. (see olafurpg/repro-scalafmt#1)

When running it twice in different sbt sessions, my whole project passes the checks.

@tanishiking
Copy link
Member

tanishiking commented Oct 14, 2018

I took a stub at this issue and found that these regressions are caused by idempotency violations (not an issue of sbt-scalafmt)

When running it twice in different sbt sessions, my whole project passes the checks.

The cause of this phenomenon is an idempotency violation and sbt.
Since sbt seems cache the unmanagedSources in scalafmt in a same session, scalafmtCheck after running scalafmt twice in a same sbt session will fail because of idempotency violation.
(On the other hand, we can pass scalafmtCheck after we run scalafmt twice in the different sbt sessions because the cache is purged on second scalafmt (in different sbt session))


By the way, I tried to fix the violations reported in #1231 (comment)

<<< test
object ViewConfigParser {
  val page: P[Page] = P( pageMode ~/ (pageSeparator ~/ nodeIdList ~ (pageSeparator ~ nodeIdList).?).? ~/ (urlSeparator | End) ).
  map {
    case x => Page()
  }
}
>>>
object ViewConfigParser {
  val page: P[Page] =
    P(pageMode ~/ (pageSeparator ~/ nodeIdList ~ (pageSeparator ~ nodeIdList).?).? ~/ (urlSeparator | End))
      .map {
        case x => Page()
      }
}

and got the following diff

[info] - newdefault/ColumnWidth.stat: test                                     | *** FAILED ***
[info]   Idempotency violated
[info]
[info]   ===========
[info]   => Obtained
[info]   ===========
[info]   object ViewConfigParser {
[info]     val page: P[Page] = P(
[info]       pageMode ~/ (pageSeparator ~/ nodeIdList ~ (pageSeparator ~ nodeIdList).?).? ~/ (urlSeparator | End)
[info]     ).map {
[info]       case x => Page()
[info]     }
[info]   }
[info]
[info]
[info]   =======
[info]   => Diff
[info]   =======
[info]      ).map {
[info]   -    case x => Page()
[info]   -  }
[info]   +      case x => Page()
[info]   +    }
[info]    } (FormatTests.scala:79)

I know the redundant indentations are caused by

(indentation for P( ... ) which has too long columns add redundant indentation indentation on case x => Page() and }), but the logic for caluculating the indentation is too complicated for me to fix the idempotency violation without breaking other test cases...
As #1278 (review), we may we want to invest our efforts in #917

@dwijnand
Copy link
Contributor

Is this still present in 2.0.0-RC4?

@joan38
Copy link
Contributor

joan38 commented Feb 11, 2019

@dwijnand 2.0.0-RC4 of what?

@dwijnand
Copy link
Contributor

Scalafmt

@joan38
Copy link
Contributor

joan38 commented Mar 7, 2019

Indeed I didn't notice that 2.x RC series were out.
Anybody knows if it's fixed?

@Krever
Copy link

Krever commented May 30, 2019

I just run into it with plugin in version 2.0.0 and version = "2.0.0-RC6" in conf file. Gladly executing clean solved the issue.

@poslegm
Copy link
Collaborator

poslegm commented Nov 8, 2019

Is it still actual? I can't reproduce

@fdietze
Copy link
Author

fdietze commented Feb 4, 2020

Not sure, I'm not using scalafmt at the moment. And if you can't reproduce, it might be a sign that this issue is fixed. :)

@poslegm poslegm closed this as completed Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants