Skip to content

Add -Xlint:stars-align or make it illegal #16741

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

Open
som-snytt opened this issue Jan 22, 2023 · 3 comments
Open

Add -Xlint:stars-align or make it illegal #16741

som-snytt opened this issue Jan 22, 2023 · 3 comments

Comments

@som-snytt
Copy link
Contributor

Compiler version

3.2.1

Minimized code

Welcome to Scala 3.2.1 (19, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> object X { def unapplySeq(ns: List[Int]): Option[(Int, Seq[Int])] = Some((ns.head, ns.tail)) }
// defined object X

scala> List(1,2,3,4) match { case X(h, t*) => t case _ => Nil }
val res0: Seq[Int] = List(2, 3, 4)

scala> List(1,2,3,4) match { case X(h, m, t*) => t case _ => Nil }
val res1: Seq[Int] = List(3, 4)

Output

It allows more extracted elements than seems to be defined on the extractor.

This is not allowed on construction:

scala> List(1, 2, List(3, 4): _*)
                            ^
       error: Sequence argument type annotation `: _*` cannot be used here:
       it is not the only argument to be passed to the single repeated parameter Int*

Expectation

➜  ~ scala -Xlint:stars-align
Welcome to Scala 2.13.10 (OpenJDK 64-Bit Server VM, Java 19).
Type in expressions for evaluation. Or try :help.

scala> object X { def unapplySeq(ns: List[Int]): Option[(Int, Seq[Int])] = Some((ns.head, ns.tail)) }
object X

scala> List(1,2,3,4) match { case X(h, m, t @ _*) => (m, t) case _ => (0, Nil) }
                                  ^
       warning: Sequence wildcard (_*) does not align with repeated case parameter or extracted sequence; the result may be unexpected.
val res0: (Int, Seq[Int]) = (2,List(3, 4))
@som-snytt som-snytt added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 22, 2023
@odersky
Copy link
Contributor

odersky commented Jan 23, 2023

I am not sure we want to flag this; it seems to be quite useful to have.

The reason we don't support it on construction is that prepend on Seq literals is not efficient. But that does not apply for extractors.

@odersky odersky added itype:enhancement itype:question and removed itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 23, 2023
@som-snytt
Copy link
Contributor Author

The edge case was scala/bug#7623

Apparently the issue was that the extractor has a different shape from the constructor.

https://javadoc.io/doc/org.scala-lang.modules/scala-xml_2.13/latest/scala/xml/Elem$.html

An interesting example from the old ticket:

scala> val r = "(a*)(b*)(c*)".r
r: scala.util.matching.Regex = (a*)(b*)(c*)

scala> "abc" match { case r(as, rest @ _*) => as }  // dangerous?
res5: String = a

scala> "abc" match { case r(as, _, _) => as }
res6: String = a

Maybe the warnable part is "extractor at odds with constructor", but of course extractors are at liberty to extract whatever.

The OP on the ticket says that the mistake was to define unapplySeq instead of unapply.

Maybe the narrower warning is about "extractor at odds with constructor with trailing sequence".

The regex example is a different issue, whether it should unapply for improved warnings.

@som-snytt
Copy link
Contributor Author

The flexargs SIP is at scala/improvement-proposals#105 which does not tweak patterns but explains intuitions and states of the arts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants