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

bugfix: Show exception from path matcher correctly #3729

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

tgodzik
Copy link
Contributor

@tgodzik tgodzik commented Dec 8, 2023

Previously, we would just get a stack trace:

Exception in thread "main" java.lang.IllegalArgumentException
	at sun.nio.fs.UnixFileSystem.getPathMatcher(UnixFileSystem.java:286)
	at org.scalafmt.config.ProjectFiles$FileMatcher$Nio.<init>(ProjectFiles.scala:74)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.$anonfun$nio$1(ProjectFiles.scala:70)
	at scala.collection.immutable.List.map(List.scala:250)
	at scala.collection.immutable.List.map(List.scala:79)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.create(ProjectFiles.scala:69)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.nio(ProjectFiles.scala:70)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.apply(ProjectFiles.scala:64)
	at org.scalafmt.cli.ScalafmtCoreRunner$.$anonfun$run$2(ScalafmtCoreRunner.scala:27)
	at metaconfig.Configured$ConfiguredImplicit.fold(Configured.scala:111)
	at org.scalafmt.cli.ScalafmtCoreRunner$.run(ScalafmtCoreRunner.scala:23)
	at org.scalafmt.cli.Cli$.runWithRunner(Cli.scala:140)
	at org.scalafmt.cli.Cli$.run(Cli.scala:91)
	at org.scalafmt.cli.Cli$.mainWithOptions(Cli.scala:61)
	at org.scalafmt.cli.Cli$.main(Cli.scala:43)
	at org.scalafmt.cli.Cli.main(Cli.scala)

Reported in VirtusLab/scala-cli#2415

@tgodzik tgodzik requested a review from kitbellew January 2, 2024 13:00
@tgodzik tgodzik force-pushed the path-matcher-error branch 2 times, most recently from a3fc5a9 to 69b9131 Compare January 2, 2024 14:29
@tgodzik
Copy link
Contributor Author

tgodzik commented Jan 2, 2024

btw. I think using regex: filter will not work currently? use excludePathswithregex: prefix but we only use nio() on excludePaths currently.

@kitbellew
Copy link
Collaborator

btw. I think using regex: filter will not work currently? use excludePathswithregex: prefix but we only use nio() on excludePaths currently.

this i didn't fully understand. nio() uses FileSystem.getPathMatcher which supports glob: and regex: prefixes.

@tgodzik
Copy link
Contributor Author

tgodzik commented Jan 2, 2024

btw. I think using regex: filter will not work currently? use excludePathswithregex: prefix but we only use nio() on excludePaths currently.

this i didn't fully understand. nio() uses FileSystem.getPathMatcher which supports glob: and regex: prefixes.

Ach, ok I didn't know nio also supported regexes

Copy link
Collaborator

@kitbellew kitbellew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, only that leftover blank line :)

catch {
case _: IllegalArgumentException =>
throw new ScalafmtConfigException(
s"Illegal pattern in configuration: $pattern"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if we append message from the exception we caught? will it explain the reason better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried and it seems empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ach, actually for Regex we can add it, it's null for Nio matcher

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added reason: ... line

Previously, we would just get a stack trace:
```
Exception in thread "main" java.lang.IllegalArgumentException
	at sun.nio.fs.UnixFileSystem.getPathMatcher(UnixFileSystem.java:286)
	at org.scalafmt.config.ProjectFiles$FileMatcher$Nio.<init>(ProjectFiles.scala:74)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.$anonfun$nio$1(ProjectFiles.scala:70)
	at scala.collection.immutable.List.map(List.scala:250)
	at scala.collection.immutable.List.map(List.scala:79)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.create(ProjectFiles.scala:69)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.nio(ProjectFiles.scala:70)
	at org.scalafmt.config.ProjectFiles$FileMatcher$.apply(ProjectFiles.scala:64)
	at org.scalafmt.cli.ScalafmtCoreRunner$.$anonfun$run$2(ScalafmtCoreRunner.scala:27)
	at metaconfig.Configured$ConfiguredImplicit.fold(Configured.scala:111)
	at org.scalafmt.cli.ScalafmtCoreRunner$.run(ScalafmtCoreRunner.scala:23)
	at org.scalafmt.cli.Cli$.runWithRunner(Cli.scala:140)
	at org.scalafmt.cli.Cli$.run(Cli.scala:91)
	at org.scalafmt.cli.Cli$.mainWithOptions(Cli.scala:61)
	at org.scalafmt.cli.Cli$.main(Cli.scala:43)
	at org.scalafmt.cli.Cli.main(Cli.scala)
```

Reported in VirtusLab/scala-cli#2415
@tgodzik tgodzik merged commit 6f65f91 into scalameta:master Jan 2, 2024
9 checks passed
@tgodzik tgodzik deleted the path-matcher-error branch January 2, 2024 16:00
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 this pull request may close these issues.

2 participants