Emit advice that compiles #8770
Conversation
| @@ -61,8 +61,8 @@ it has 13 unimplemented members. | |||
|
|
|||
| class Baz[T] extends Collection[T] | |||
| ^ | |||
| abstract-report2.scala:15: error: class Dingus needs to be abstract, since: | |||
| it has 24 unimplemented members. | |||
| abstract-report2.scala:21: error: class Dingus needs to be abstract, since: | |||
There was a problem hiding this comment.
OT/apropos of nothing, I noticed the since: is weird because of deprecation messaging.
There was a problem hiding this comment.
So it is. Also, why the newline?
| // Members declared in Symbolic | ||
| def --!(i: Int): Unit = ??? | ||
| def --? : Int = ??? | ||
| def unary_~ : Long = ??? |
|
I was hoping @som-snytt was going to approve without waiting for the build, but he's shrewder than that. Object {
final private val x: Int
def x(): Int
- def <init>(): Object{def x(): Int}
+ def <init>(): $anon$1
} |
|
@hrhino A task came up on my day job, I'll get back to it in a bit. I did single comments so you wouldn't have to wait for the actual review. |
| ) | ||
| else if (isModule) "" // avoid "object X of type X.type" | ||
| else tp match { | ||
| case PolyType(tparams, res) => typeParamsString(tp) + loop(res) |
There was a problem hiding this comment.
My last thought before the vp asked me to look at something was that this will trigger -Xlint:recurse-with-default when merged forward.
There was a problem hiding this comment.
Am I making life interesting for @SethTisue or whoever does the forward merges, or am I just being irritating? AFAIR we don't use that flag so maybe it's a pending enhancement for your next tranche of warnings fixups.
There was a problem hiding this comment.
That's the flag you approved in January on 2.13.
| else tp.resultType match { | ||
| case rt @ TypeBounds(_, _) => "" + rt | ||
| case rt => " <: " + rt | ||
| def loop(tp: Type, first: Boolean = false): String = { |
There was a problem hiding this comment.
You know what's clever? Shadowing an outer param in an inner loop, so it looks like the whole thing only has one param.
You know what's wrong?
|
Oh good, it passed. Let me know if you like the version with aligned arrows. I also renamed the boolean. Otherwise LGTM. |
Yeah, what's up with that? You care about whitespaces, but then you don't? Signed, a-vertical-alignment-lover. |
|
The urban dictionary definition of vertical alignment lover is when you find someone your own height. It probably means something else, too. |
hrhino
force-pushed
the
t9908
branch
3 times, most recently
from
4cf3f02
to
a8f7289
Compare
Like it asks for on the ticket. Presumably there's a lot more advice that doesn't compile, such as scala/bug#11301, but here's a step forward. Fixes scala/bug#9908. Tweaked, reformatted, and generally Co-Authored-By: "Som Snytt" <som.snytt@gmail.com>
|
@som-snytt I did like it, thanks. I hope you don't mind me squashing your commit. I gave you @dwijnand I too am a vertical-alignment lover! (And also a compound-adjective hyphenator.) IntelliJ, on the other hand, drops those spaces whenever I ask it to do something as simple as indent a new block. Please forgive me. |
|
#8776 by way of penance. |
|
Very impressive crediting, I expected just a quick cut/paste. I only started editing because I was stuck at the office and mistakenly thought the commit wasn't passing. |
|
@SethTisue Wondering how many 2.12 contributions will be orphaned by the current milestone? |
|
You can be the release lead for 2.12.11-and-3/4. |
Unfortunately we don't have tooling for using a release branch to keep 2.12.11 and 2.12.12 work separate. So, let's all please exercise a higher-than-usual level of caution about merging 2.12.x PRs until 2.12.11 is out the door, and even for a little while afterwards, in case we end up needing to rush a followup. In any case, this one seems safe. Such a small word, "seems" :-) |
Like it asks for on the ticket. Presumably there's a lot more advice that doesn't compile, such as scala/bug#11301, but here's a step forward.
Fixes scala/bug#9908.