The Wayback Machine - https://web.archive.org/web/20221215011955/https://github.com/scala/scala/pull/10239
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

Fix incorrect cast in StrictOptimizedMapOps.+ #10239

Merged
merged 1 commit into from Dec 14, 2022
Merged

Conversation

lrytz
Copy link
Member

@lrytz lrytz commented Dec 13, 2022

@scala-jenkins scala-jenkins added this to the 2.13.11 milestone Dec 13, 2022
Copy link
Contributor

@som-snytt som-snytt left a comment

Tests pass locally. The best part is adding a space after if.

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

/rebuild

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

Oh, not spurious. I couldn't see the cause

[error] 1 of 19 test tasks failed:
[error] - junit/testOnly -- +v
[error]   - junit/test:compileIncremental failed: Compilation failed
[error] Failure due to previous errors
[error] (testAll) Failure due to previous errors

but travis shows

[error] /home/travis/build/scala/scala/test/junit/scala/collection/immutable/MapTest.scala:149:25: multiarg infix syntax looks like a tuple and will be deprecated

[error]     assertEquals(7, (m1 + (2 -> 2, 3 -> 3, 4 -> 4, 5 -> 5, 6 -> 6, 7 -> 7)).size)

[error]                         ^

[error] one error found

Ha, ha.

Fixable by adding @deprecated to test method.

Could also remove extra space List( 4 -> 4, while editing.
Could also remove parens from test method, as that does not warn in test.

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

Travis also reports downloading sbt launcher 1.6.2. Not sure at what point that matters. TIL testOnly -- +v.

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

Edit: I tried deprecating the test method and it still errors out locally. At least TIL set Global / fatalWarnings := true which FSR isn't Werror.

Oh I didn't read the message carefully:

context.warning(tree.pos, "multiarg infix syntax looks like a tuple and will be deprecated", WarningCategory.LintMultiargInfix)

This was a pre-deprecation warning. I'll try @predeprecated.

Also this makes my blood boil every time:

MapTest.scala:145:4: not found: type nowarn

Of course it's also necessary to suppress the deprecation of the method.

@@ -141,4 +141,11 @@ class MapTest {
}
}
}

Copy link
Contributor

@som-snytt som-snytt Dec 13, 2022

Choose a reason for hiding this comment

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

Suggested change
@deprecated("tests deprecated API", "")
@nowarn("cat=lint-multiarg-infix")

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

Probably lrytz didn't notice that I added a commit...

I assumed he was asleep, I didn't check the time zones.

I must have done it wrong: https://scala-ci.typesafe.com/job/scala-2.13.x-validate-main/16757/

+import annotation._
+
 import org.junit.Assert.assertEquals
 import org.junit.Test

@@ -142,10 +144,12 @@ class MapTest {
     }
   }

+  @deprecated("tests deprecated API", since="2.13.11")
+  @nowarn("cat=lint-multiarg-infix")
   @Test
-  def t12699(): Unit = {
+  def t12699: Unit = {
     val m1: HashMap[Int, Int] = HashMap(1 -> 1)
-    assertEquals(7, m1.+(elem1 = 2 -> 2, elem2 = 3 -> 3, elems = List( 4 -> 4, 5 -> 5, 6 -> 6, 7 -> 7): _*).size)
+    assertEquals(7, m1.+(elem1 = 2 -> 2, elem2 = 3 -> 3, elems = List(4 -> 4, 5 -> 5, 6 -> 6, 7 -> 7): _*).size)
     assertEquals(7, (m1 + (2 -> 2, 3 -> 3, 4 -> 4, 5 -> 5, 6 -> 6, 7 -> 7)).size)
   }
 }

@lrytz
Copy link
Member Author

lrytz commented Dec 13, 2022

I'm sorry :-/ I saw the fatal warning three hours ago and now just came back to fix it without paying attention. Feel free to push -f e876cb7 again!

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

I don't understand how it passes with your nowarn, so I won't attempt it again. Later, I'll try to figure it out, maybe after I do some advent of code.

@lrytz
Copy link
Member Author

lrytz commented Dec 13, 2022

I'm using m1.+ instead of infix m1 +, the nowarn is for + being deprecated...

@som-snytt
Copy link
Contributor

som-snytt commented Dec 13, 2022

Thanks, my eyes were too bleary to see it.

@lrytz lrytz merged commit 1045255 into scala:2.13.x Dec 14, 2022
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.