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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import util.control.Breaks
import org.scalafmt.Error
import org.scalafmt.{Formatted, Scalafmt, Versions}
import org.scalafmt.config.{ProjectFiles, ScalafmtConfig}
import org.scalafmt.config.ScalafmtConfigException
import org.scalafmt.CompatCollections.ParConverters._

import scala.meta.parsers.ParseException
Expand All @@ -21,10 +22,18 @@ object ScalafmtCoreRunner extends ScalafmtRunner {
ExitCode.UnexpectedError
} { scalafmtConf =>
options.common.debug.println(s"parsed config (v${Versions.version})")
val filterMatcher = ProjectFiles.FileMatcher(
scalafmtConf.project,
options.customExcludes
)
val filterMatcher =
try {
ProjectFiles.FileMatcher(
scalafmtConf.project,
options.customExcludes
)
} catch {
case e: ScalafmtConfigException =>
options.common.err.println(e.getMessage())
return ExitCode.UnexpectedError // RETURNING EARLY!
}

val inputMethods = getInputMethods(options, filterMatcher.matchesPath)

val termDisplay =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,27 @@ object ProjectFiles {
private def regex(seq: Seq[String]) = create(seq, new Regex(_))

private final class Nio(pattern: String) extends PathMatcher {
private val matcher = fs.getPathMatcher(pattern)
private val matcher =
try { fs.getPathMatcher(pattern) }
catch {
case _: IllegalArgumentException =>
throw new ScalafmtConfigException(
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
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

)
}
def matches(path: file.Path): Boolean = matcher.matches(path)
}
private final class Regex(regex: String) extends PathMatcher {
private val pattern = java.util.regex.Pattern.compile(regex)
private val pattern =
try { java.util.regex.Pattern.compile(regex) }
catch {
case e: java.util.regex.PatternSyntaxException =>
throw new ScalafmtConfigException(
s"""|Illegal regex in configuration: $regex
|reason: ${e.getMessage()}""".stripMargin
)
}

def matches(path: file.Path): Boolean =
pattern.matcher(path.toString).find()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,37 @@ class DynamicSuite extends FunSuite {
)
}

check("regex-error") { f =>
f.setConfig(s"""|version = "$nightly"
|runner.dialect = "scala212"
|project.excludeFilters = [
| ".*foo("
|]
|""".stripMargin)
val err = f.assertThrows[ScalafmtDynamicError.ConfigParseError]().getMessage
assertNoDiff(
err.takeRight(120),
"""|Invalid config: Illegal regex in configuration: .*foo(
|reason: Unclosed group near index 6
|.*foo(
|""".stripMargin
)
}

check("path-error") { f =>
f.setConfig(s"""|version = "$nightly"
|runner.dialect = "scala212"
|project.excludePaths = [
| "foo.scala"
|]
|""".stripMargin)
val err = f.assertThrows[ScalafmtDynamicError.ConfigParseError]().getMessage
assertNoDiff(
err.takeRight(120),
"Invalid config: Illegal pattern in configuration: foo.scala"
)
}

check("config-cache") { f =>
f.setVersion(latest, "scala211")
f.assertFormat()
Expand Down
58 changes: 58 additions & 0 deletions scalafmt-tests/src/test/scala/org/scalafmt/cli/CliTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,64 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
testCli("1.6.0-RC4") // test for runDynamic
testCli(stableVersion) // test for runScalafmt

test(s"path-error") {
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
val input =
s"""|/.scalafmt.conf
|version = "${stableVersion}"
|project.excludePaths = [
| "glob:**/src/main/scala/besom/rpc/**.scala",
| "foo.scala"
|]
|/foo.scala
|object A { foo( }
|""".stripMargin

noArgTest(
string2dir(input),
input,
Seq(Array("--test")),
ExitCode.UnexpectedError,
assertOut = out => {
assertContains(
out,
s"""Illegal pattern in configuration: foo.scala""".stripMargin
)
},
Some(ExitCode.UnexpectedError)
)

}

test(s"regex-error") {
val input =
s"""|/.scalafmt.conf
|version = "${stableVersion}"
|project.excludeFilters = [
| ".*foo("
|]
|/foo.scala
|object A { foo( }
|""".stripMargin

noArgTest(
string2dir(input),
input,
Seq(Array("--test")),
ExitCode.UnexpectedError,
assertOut = out => {
assertContains(
out,
"""|Illegal regex in configuration: .*foo(
|reason: Unclosed group near index 6
|.*foo(
|""".stripMargin
)
},
Some(ExitCode.UnexpectedError)
)

}

test("Fail if .scalafmt.conf is missing.") {
val input =
s"""|/foo.scala
Expand Down
Loading