From 6a842ae9129870efe1eb1e0e96cf62ae64b2403e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=CC=81bastien=20Boulet?= Date: Mon, 19 Oct 2020 18:05:22 +0200 Subject: [PATCH 1/4] check the whole caret position in the testkit --- .../scalafix/internal/testkit/AssertDelta.scala | 2 ++ .../internal/testkit/CommentAssertion.scala | 14 ++++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala index 29bec6a22..aeec51c16 100644 --- a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala +++ b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala @@ -26,6 +26,8 @@ case class AssertDelta( (assert.caretPosition match { case Some(carPos) => (carPos.start == lintDiagnostic.position.start) && + (carPos.end == lintDiagnostic.position.end || + (carPos.start == (carPos.end - 1) && lintDiagnostic.position.start == lintDiagnostic.position.end)) && assert.expectedMessage.forall(_.trim == lintDiagnostic.message.trim) case None => sameLine(assert.anchorPosition) diff --git a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala index a2c8e9760..01968f689 100644 --- a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala +++ b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala @@ -97,19 +97,21 @@ object MultiLineAssertExtractor { ) } - val caretOffset = lines(1).indexOf('^') - if (caretOffset == -1) + val caretOffsetStart = lines(1).indexOf('^') + if (caretOffsetStart == -1) throw new Exception("^ should be on the second line of the assert") - val offset = caretOffset - token.pos.startColumn - val caretStart = token.pos.start + offset + val caretOffsetEnd = lines(1).lastIndexOf('^') + val offsetStart = caretOffsetStart - token.pos.startColumn + val offsetEnd = caretOffsetEnd - token.pos.startColumn + 1 + val caretStart = token.pos.start + offsetStart + val caretEnd = token.pos.start + offsetEnd val message = lines.drop(2).mkString("\n") - Some( CommentAssertion( anchorPosition = token.pos, key = key, caretPosition = Some( - Position.Range(token.pos.input, caretStart, caretStart) + Position.Range(token.pos.input, caretStart, caretEnd) ), expectedMessage = Some(message) ) From 35d1ba61529117a95b69e963715a2279de3c60ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=CC=81bastien=20Boulet?= Date: Mon, 19 Oct 2020 18:34:06 +0200 Subject: [PATCH 2/4] update tests --- .../input/src/main/scala/test/DisableSyntaxMoreRules.scala | 6 +++--- scalafix-tests/input/src/main/scala/test/NoFinalize.scala | 2 +- .../main/scala/test/disableSyntax/DisableSyntaxBase.scala | 2 +- .../test/disableSyntax/DisableSyntaxNoValPatterns.scala | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/scalafix-tests/input/src/main/scala/test/DisableSyntaxMoreRules.scala b/scalafix-tests/input/src/main/scala/test/DisableSyntaxMoreRules.scala index 006b85a45..e3b77ace8 100644 --- a/scalafix-tests/input/src/main/scala/test/DisableSyntaxMoreRules.scala +++ b/scalafix-tests/input/src/main/scala/test/DisableSyntaxMoreRules.scala @@ -24,7 +24,7 @@ object DisableSyntaxMoreRules { class Foo { def bar(x: Int = 42) = 1 /* assert: DisableSyntax.defaultArgs - ^ + ^^ Default args makes it hard to use methods as functions. */ } @@ -58,11 +58,11 @@ Default args makes it hard to use methods as functions. class Bar { type Foo = Int; def foo = 42 } def foo(a: { type Foo = Int; def foo: Foo } = new Bar): Int = a.foo /* assert: DisableSyntax.defaultArgs - ^ + ^^^^^^^ Default args makes it hard to use methods as functions. */ def foo2(a: { type Foo = Int; def foo: Foo } = {val a = new Bar; a}): Int = a.foo /* assert: DisableSyntax.defaultArgs - ^ + ^^^^^^^^^^^^^^^^^^^^ Default args makes it hard to use methods as functions. */ diff --git a/scalafix-tests/input/src/main/scala/test/NoFinalize.scala b/scalafix-tests/input/src/main/scala/test/NoFinalize.scala index e529aefed..99a67f9f2 100644 --- a/scalafix-tests/input/src/main/scala/test/NoFinalize.scala +++ b/scalafix-tests/input/src/main/scala/test/NoFinalize.scala @@ -25,7 +25,7 @@ case object NoFinalize { class Example { override protected def finalize() = () /* assert: DisableSyntax.noFinalize - ^ + ^^^^^^^^ finalize should not be used */ } diff --git a/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxBase.scala b/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxBase.scala index be4f16b8e..4dd79b268 100644 --- a/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxBase.scala +++ b/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxBase.scala @@ -36,7 +36,7 @@ case object DisableSyntaxBase { null /* assert: DisableSyntax.null - ^ + ^^^^ null should be avoided, consider using Option instead */ diff --git a/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxNoValPatterns.scala b/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxNoValPatterns.scala index f3addc4de..af5a7ddb2 100644 --- a/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxNoValPatterns.scala +++ b/scalafix-tests/input/src/main/scala/test/disableSyntax/DisableSyntaxNoValPatterns.scala @@ -14,9 +14,9 @@ case object DisableSyntaxNoValPatterns { val (works6, _) = (1, Left(42)) case class TestClass(a: Int, b: Int) val TestClass(a, b) = TestClass(1, 1) /* assert: DisableSyntax.noValPatterns - ^ + ^^^^^^^^^^^^^^^ Pattern matching in val assignment can result in match error, use "_ match { ... }" with a fallback case instead.*/ val TestClass(c, _) = TestClass(1, 1) /* assert: DisableSyntax.noValPatterns - ^ + ^^^^^^^^^^^^^^^ Pattern matching in val assignment can result in match error, use "_ match { ... }" with a fallback case instead.*/ } From 367c0044aa4d8ec0d90b17bf96ca3f992c2ec5b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=CC=81bastien=20Boulet?= Date: Mon, 19 Oct 2020 21:32:34 +0200 Subject: [PATCH 3/4] DRY: rely on Position.lineCaret --- .../main/scala/scalafix/internal/testkit/AssertDelta.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala index aeec51c16..e4d9dc483 100644 --- a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala +++ b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/AssertDelta.scala @@ -25,9 +25,7 @@ case class AssertDelta( sameKey(assert.key) && (assert.caretPosition match { case Some(carPos) => - (carPos.start == lintDiagnostic.position.start) && - (carPos.end == lintDiagnostic.position.end || - (carPos.start == (carPos.end - 1) && lintDiagnostic.position.start == lintDiagnostic.position.end)) && + carPos.lineCaret == lintDiagnostic.position.lineCaret && assert.expectedMessage.forall(_.trim == lintDiagnostic.message.trim) case None => sameLine(assert.anchorPosition) From 5856f9a5fc80d56690b64ecbd24cc8307e66983e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=CC=81bastien=20Boulet?= Date: Tue, 20 Oct 2020 11:59:06 +0200 Subject: [PATCH 4/4] update CommentAssertion documentation Replace the example in the documentation by a real one from the tests --- .../internal/testkit/CommentAssertion.scala | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala index 01968f689..32e3949ce 100644 --- a/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala +++ b/scalafix-testkit/src/main/scala/scalafix/internal/testkit/CommentAssertion.scala @@ -12,7 +12,9 @@ import scalafix.internal.util.PositionSyntax._ // For example: // // ```scala -// Option(1).get // assert: Disable.get +// class Foo { +// def bar(x: Int = 42) = 1 // assert: DisableSyntax.defaultArgs +// } // ``` // // You can also use the multiline variant. This isuseful two visually @@ -22,14 +24,13 @@ import scalafix.internal.util.PositionSyntax._ // For example: // // ```scala -// Option(1).get /* assert: Disable.get -// ^ -// Option.get is the root of all evilz -// -// If you must Option.get, wrap the code block with -// // scalafix:off Option.get -// ... -// // scalafix:on Option.get +// class Foo { +// def bar(x: Int = 42) = 1 /* assert: DisableSyntax.defaultArgs +// ^^ +// Default args makes it hard to use methods as functions. +// */ +// } +// ``` // */ case class CommentAssertion( anchorPosition: Position,