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

Scalafix does not work on Windows: illegal/unsupported escape sequence #583

Closed
OlegYch opened this issue Jan 30, 2018 · 18 comments
Closed
Assignees
Labels
Milestone

Comments

@OlegYch
Copy link
Contributor

OlegYch commented Jan 30, 2018

using sbt 1.1.0 on windows
scalafixSettings
scalafixConfigure(Compile, Test)
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.5.10")

sbt:root> scalafixCli --verbose
[info] Running scalafix --config D:\.scalafix.conf --verbose --classpath D:\api\target\scala-2.12\classes;D:\api\target\scala-2.12\test-classes --sourceroot D:\ error: Invalid '(D:\<snip>api\test\helpers\BaseServerTest.scala)' for  --include/--exclude. Illegal/unsupported escape sequence near index 35
(D:\<snip>api\test\helpers\BaseServerTest.scala)
[error] scalafix.cli.Cli$NonZeroExitCode: Got exit code InvalidCommandLineOption=8
@olafurpg olafurpg added the bug label Jan 30, 2018
@olafurpg
Copy link
Contributor

Thank you for reporting! I have tracked down the error to this location

private val resolvedPathMatcher: Configured[FilterMatcher] = try {
val include = if (files.isEmpty) List(".*") else files
Ok(FilterMatcher(include, exclude))
} catch {
case e: PatternSyntaxException =>
ConfError
.message(
s"Invalid '${e.getPattern}' for --include/--exclude. ${e.getMessage}")
.notOk
}

It seems we are using the filenames as a filter to include/exclude files. This looks very weird to me, fix is likely simple. We should first setup CI on appveyor to reproduce the bug.

@MasseGuillaume MasseGuillaume self-assigned this Jan 31, 2018
@MasseGuillaume
Copy link
Contributor

I got the CI setup: #585

The next step is to find a Windows machine :).

This was referenced Jan 31, 2018
@aumann
Copy link

aumann commented Apr 4, 2018

Is there any known workaround for this issue?

@olafurpg
Copy link
Contributor

olafurpg commented Apr 9, 2018

There is no known workaround @aumann We have a decent backlog of tickets that need to be addressed and this is one of my top priorities once I get to cleaning up our issue tracker. Past months have been primarily focused on architectural changes that are partly addressed in #675

@ochrons
Copy link

ochrons commented Apr 23, 2018

First time I tried running scalafix I ran right into this problem. Not a superb initial experience with the (potentially great?) tool :)

@olafurpg
Copy link
Contributor

😭 I know...

The reason I'm pushing this back is because the upcoming scalafix release will be so much better than the current 0.5.10, but I'm blocked by a couple more critical changes. I would prefer if you hold onto trying scalafix until v0.6.0 is out 😊 I'm sorry!

@olafurpg
Copy link
Contributor

Even if I fixed this in master today you'd have to wait a couple more weeks until a stable release is out.

@olafurpg olafurpg added this to the v0.6.0-RC1 milestone Apr 23, 2018
@ochrons
Copy link

ochrons commented Apr 23, 2018

As a workaround I can run sbt and scalafix under Windows WSL (so in Linux basically).

@olafurpg
Copy link
Contributor

Thanks for the workaround!

The user experience is still pretty rough in v0.5 (memory consumption is way too high, for example), I'm working towards making it smoother. There's been a lot of R&D for the past ~1.5 year in how to deliver semantic tooling outside of the compiler and I'm finally happy with what we have in scalafix, I will make more noise once I think it's proper good :)

@ochrons
Copy link

ochrons commented Apr 23, 2018

Was able to run Cats 1.0.0 migration with Scalafix using WSL workaround. But looking forward into using scalafix as a linting tool and for that I need it to work in Windows directly. Can wait a few weeks though 😃

@olafurpg
Copy link
Contributor

Sweet, glad it worked out for the cats 1.0 migration, hope it was helpful!

This ticket will be my first priority as I'm done the refactoring of our APIs.

@olafurpg olafurpg assigned olafurpg and unassigned MasseGuillaume Apr 23, 2018
@olafurpg olafurpg changed the title Illegal/unsupported escape sequence Scalafix does not work on Windows: illegal/unsupported escape sequence Apr 25, 2018
@olafurpg
Copy link
Contributor

It seems the best solution to fix this is to use java.nio PatchMatcher https://docs.oracle.com/javase/tutorial/essential/io/find.html

Scalafix already requires JDK 8.

@olafurpg
Copy link
Contributor

Scalafix v0.6.0-M7 is out and I suspect this bug is now fixed. To try it out, it is necessary to add the following to your build

addCompilerPlugin(scalafixSemanticdb)
scalacOptions += "-Yrangepos

sbt-scalafix used to add those settings automatically but not work for all builds so this manual step is now required instead.

Note that the website docs are for 0.5.10. v0.6 will remain in milestone phase for about a month longer, you should not encounter a regression by upgrading unless you use ExplicitResultTypes.

@olafurpg
Copy link
Contributor

olafurpg commented Aug 3, 2018

I believe the Windows-related issues have now been fixed. Our CI runs both unit tests and full integration tests for the sbt plugin scalacenter/sbt-scalafix#10

I hope to release Scalafix v0.6 stable before the end of August!

@olafurpg olafurpg closed this as completed Aug 3, 2018
@melezov
Copy link

melezov commented Oct 9, 2018

Looking forward to the 0.6 release!

@olafurpg
Copy link
Contributor

olafurpg commented Oct 9, 2018

@melezov it is already released as scalafix v0.9.0. Please give it a try and let me know how it works :)

@melezov
Copy link

melezov commented Oct 9, 2018

It works phenomenally. I can finally toss away the nux VBox which I kept around for the purpose of scalafixing. Also, it takes approx 10 minutes to run RemoveUnused while it previously took ~ 60 mins for RemoveUnusedImports on 0.5.10 - great work

@olafurpg
Copy link
Contributor

Great to hear @melezov, thanks for reporting back! I would love to hear if you notice further speedups after we release the optimizations in #898 😉

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

No branches or pull requests

6 participants