From 96f8d009c0e3929fe55311f3f052e32cc30e024c Mon Sep 17 00:00:00 2001 From: Matthew de Detrich Date: Mon, 24 Jul 2023 10:06:06 +0200 Subject: [PATCH 1/5] Try to parse all valid Testkit HOCON comments --- .../scala/scalafix/testkit/RuleTest.scala | 71 +++++++++++-------- .../scalafix/testkit/SemanticRuleSuite.scala | 10 ++- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala index 752996d32..a789a40f0 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala @@ -31,36 +31,47 @@ object RuleTest { val input = test.toInput val dialect = args.scalaVersion.dialect(args.sourceScalaVersion) val tree = dialect(input).parse[Source].get - val comment = SemanticRuleSuite.findTestkitComment(tree.tokens) - val syntax = comment.syntax.stripPrefix("/*").stripSuffix("*/") - val conf = Conf.parseString(test.testName, syntax).get - val scalafixConfig = conf.as[ScalafixConfig].get - val doc = v1.SyntacticDocument( - tree.pos.input, - LazyValue.now(tree), - DiffDisable.empty, - scalafixConfig - ) - val sdoc = - v1.SemanticDocument.fromPath( - doc, - test.semanticdbPath, - classLoader, - symtab - ) - val decoderSettings = - RuleDecoder.Settings().withConfig(scalafixConfig) - val decoder = RuleDecoder.decoder(decoderSettings) - val rulesConf = ConfGet - .getKey(conf, "rules" :: "rule" :: Nil) - .getOrElse(Conf.Lst(Nil)) - val config = Configuration() - .withConf(conf) - .withScalaVersion(args.scalaVersion.value) - .withScalacOptions(args.scalacOptions) - .withScalacClasspath(args.classpath.entries) - val rules = decoder.read(rulesConf).get.withConfiguration(config).get - (rules, sdoc) + // Its possible to have comments which isn't valid HOCON (i.e. + // license headers), so lets parse until we find valid HOCON. + val allComments = SemanticRuleSuite.filterPossibleTestkitComments(tree.tokens) + val lazilyParsedComments = allComments.view.map{ comment => + val syntax = comment.syntax.stripPrefix("/*").stripSuffix("*/") + for { + conf <- Conf.parseString(test.testName, syntax).toEither + scalafixConfig <- conf.as[ScalafixConfig].toEither + } yield { + val doc = v1.SyntacticDocument( + tree.pos.input, + LazyValue.now(tree), + DiffDisable.empty, + scalafixConfig + ) + val sdoc = + v1.SemanticDocument.fromPath( + doc, + test.semanticdbPath, + classLoader, + symtab + ) + val decoderSettings = + RuleDecoder.Settings().withConfig(scalafixConfig) + (RuleDecoder.decoder(decoderSettings), conf, sdoc) + } + } + + lazilyParsedComments.collectFirst { + case Right((decoder, conf, sdoc)) => + val rulesConf = ConfGet + .getKey(conf, "rules" :: "rule" :: Nil) + .getOrElse(Conf.Lst(Nil)) + val config = Configuration() + .withConf(conf) + .withScalaVersion(args.scalaVersion.value) + .withScalacOptions(args.scalacOptions) + .withScalacClasspath(args.classpath.entries) + val rules = decoder.read(rulesConf).get.withConfiguration(config).get + (rules, sdoc) + }.getOrElse(throw new IllegalArgumentException(s"Expected a single comment at ${test.testPath} with a valid rule or rules key defined")) } new RuleTest(test, run) diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/SemanticRuleSuite.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/SemanticRuleSuite.scala index 04644da4a..2a93579c7 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/SemanticRuleSuite.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/SemanticRuleSuite.scala @@ -28,11 +28,12 @@ object SemanticRuleSuite { }.mkString } + private[testkit] def testKitCommentPredicate(token: Token): Boolean = + token.is[Token.Comment] && token.syntax.startsWith("/*") + def findTestkitComment(tokens: Tokens): Token = { tokens - .find { x => - x.is[Token.Comment] && x.syntax.startsWith("/*") - } + .find(testKitCommentPredicate) .getOrElse { val input = tokens.headOption.fold("the file")(_.input.syntax) throw new IllegalArgumentException( @@ -41,4 +42,7 @@ object SemanticRuleSuite { } } + def filterPossibleTestkitComments(tokens: Tokens): IndexedSeq[Token] = + tokens.filter(testKitCommentPredicate) + } From 5d2a98f9bba379883102f4ea146cdfa6de50441d Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Sat, 9 Sep 2023 21:14:44 +0200 Subject: [PATCH 2/5] scalafmt --- .../main/scala/scalafix/testkit/RuleTest.scala | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala index a789a40f0..2aec66418 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala @@ -33,8 +33,9 @@ object RuleTest { val tree = dialect(input).parse[Source].get // Its possible to have comments which isn't valid HOCON (i.e. // license headers), so lets parse until we find valid HOCON. - val allComments = SemanticRuleSuite.filterPossibleTestkitComments(tree.tokens) - val lazilyParsedComments = allComments.view.map{ comment => + val allComments = + SemanticRuleSuite.filterPossibleTestkitComments(tree.tokens) + val lazilyParsedComments = allComments.view.map { comment => val syntax = comment.syntax.stripPrefix("/*").stripSuffix("*/") for { conf <- Conf.parseString(test.testName, syntax).toEither @@ -59,8 +60,8 @@ object RuleTest { } } - lazilyParsedComments.collectFirst { - case Right((decoder, conf, sdoc)) => + lazilyParsedComments + .collectFirst { case Right((decoder, conf, sdoc)) => val rulesConf = ConfGet .getKey(conf, "rules" :: "rule" :: Nil) .getOrElse(Conf.Lst(Nil)) @@ -71,7 +72,12 @@ object RuleTest { .withScalacClasspath(args.classpath.entries) val rules = decoder.read(rulesConf).get.withConfiguration(config).get (rules, sdoc) - }.getOrElse(throw new IllegalArgumentException(s"Expected a single comment at ${test.testPath} with a valid rule or rules key defined")) + } + .getOrElse( + throw new IllegalArgumentException( + s"Expected a single comment at ${test.testPath} with a valid rule or rules key defined" + ) + ) } new RuleTest(test, run) From fd35cf2b6e63fb251e73a152a0badede1b93ab75 Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Sat, 9 Sep 2023 21:14:50 +0200 Subject: [PATCH 3/5] add test --- .../input/src/main/scala-2/test/Headers.scala | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 scalafix-tests/input/src/main/scala-2/test/Headers.scala diff --git a/scalafix-tests/input/src/main/scala-2/test/Headers.scala b/scalafix-tests/input/src/main/scala-2/test/Headers.scala new file mode 100644 index 000000000..72a1dfa07 --- /dev/null +++ b/scalafix-tests/input/src/main/scala-2/test/Headers.scala @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * license agreements; and to You under the Apache License, version 2.0: + * + * https://www.apache.org/licenses/LICENSE-2.0 + */ + +/* + * Copyright (C) xxxxxxx + */ + +/* +rules = [ + "class:scalafix.test.NoDummy" +] +*/ +package test + +class Headers { + val aDummy = 0 // assert: NoDummy +} + From 44aff592617fa288a893a6ffacf720adf4af2512 Mon Sep 17 00:00:00 2001 From: Brice Jaglin Date: Sun, 10 Sep 2023 10:16:15 +0200 Subject: [PATCH 4/5] ignore comments that metadoc fails on --- .../src/main/scala/scalafix/testkit/RuleTest.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala index 2aec66418..6ad59add4 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala @@ -1,5 +1,7 @@ package scalafix.testkit +import scala.util.Try + import scala.meta._ import scala.meta.internal.symtab.SymbolTable @@ -38,7 +40,8 @@ object RuleTest { val lazilyParsedComments = allComments.view.map { comment => val syntax = comment.syntax.stripPrefix("/*").stripSuffix("*/") for { - conf <- Conf.parseString(test.testName, syntax).toEither + conf <- Try(Conf.parseString(test.testName, syntax)).toEither + .flatMap(_.toEither) scalafixConfig <- conf.as[ScalafixConfig].toEither } yield { val doc = v1.SyntacticDocument( From a92693d35001970bd7c5afee09cfd3dc3cbba3d5 Mon Sep 17 00:00:00 2001 From: Matthew de Detrich Date: Sun, 10 Sep 2023 10:36:35 +0200 Subject: [PATCH 5/5] Fix Either Left being serializable --- .../src/main/scala/scalafix/testkit/RuleTest.scala | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala index 6ad59add4..ec25d996c 100644 --- a/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala +++ b/scalafix-testkit/src/main/scala/scalafix/testkit/RuleTest.scala @@ -1,11 +1,14 @@ package scalafix.testkit +import scala.util.Failure +import scala.util.Success import scala.util.Try import scala.meta._ import scala.meta.internal.symtab.SymbolTable import metaconfig.Conf +import metaconfig.ConfError import metaconfig.internal.ConfGet import metaconfig.typesafeconfig.typesafeConfigMetaconfigParser import scalafix.internal.config.ScalafixConfig @@ -40,8 +43,10 @@ object RuleTest { val lazilyParsedComments = allComments.view.map { comment => val syntax = comment.syntax.stripPrefix("/*").stripSuffix("*/") for { - conf <- Try(Conf.parseString(test.testName, syntax)).toEither - .flatMap(_.toEither) + conf <- Try(Conf.parseString(test.testName, syntax)) match { + case Failure(exception) => Left(ConfError.exception(exception)) + case Success(value) => value.toEither + } scalafixConfig <- conf.as[ScalafixConfig].toEither } yield { val doc = v1.SyntacticDocument(