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

Deprecate and disallow abstract methods without result type in traits and abstract classes #7523

Closed
scabug opened this issue May 28, 2013 · 18 comments
Labels

Comments

@scabug
Copy link

scabug commented May 28, 2013

Judging from the lack of strong reactions against getting rid of this item in https://groups.google.com/d/topic/scala-internals/q1YX7NC1geM/discussion I think we can create an issue to track it.

trait Foo { def foo }
abstract Bar { def bar }

The fact that this is accepted by the compiler is incredibly surprising, because it goes against the intuition that the compiler needs either a body to infer the type or an explicit type annotation.

Adding to the unintuitiveness is that in most cases where a type is missing the compiler infers Nothing (for better or worse), but doesn't do it here.

The suggestion is to deprecate the syntax for 2.11 and start requiring a type in 2.12:

trait Foo { def foo: Unit }
abstract Bar { def bar: Unit }

The behaviour is described in chapter 4.6.3 of the spec. Getting rid of this special-cased behaviour would allow getting rid of the whole chapter.

@scabug
Copy link
Author

scabug commented May 28, 2013

Imported From: https://issues.scala-lang.org/browse/SI-7523?orig=1
Reporter: @soc
Affected Versions: 2.11.0-M3
See #7648, #6811

@scabug
Copy link
Author

scabug commented May 28, 2013

@soc said:
scala/scala#2604

@scabug
Copy link
Author

scabug commented May 28, 2013

@JamesIry said:
Won't this require a spec update as well? Or is this something the compiler does above and beyond the spec?

@scabug
Copy link
Author

scabug commented May 29, 2013

@soc said:
As Lukas Rytz mentioned, it special-cased in the spec. Getting rid of it would allow us to remove the whole chapter 4.6.3. I'll update the description.

@scabug
Copy link
Author

scabug commented May 29, 2013

@retronym said:
4.6.3 covers both procedure definitions and declarations.

@scabug
Copy link
Author

scabug commented May 29, 2013

@soc said:
The definition part depends on getting the rest of the fixes through, which are not covered in this ticket.

@scabug
Copy link
Author

scabug commented Jan 28, 2014

@adriaanm said:
Simon, could you look into providing an -Xlint warning -- the compromise proposed in the PR?

@scabug
Copy link
Author

scabug commented Jan 28, 2014

@adriaanm said:
See also #7212

@scabug
Copy link
Author

scabug commented Jan 28, 2014

@soc said:
@adriaan, not sure I understand. The warning is already implemented, it's just that it takes both -deprecation and -Xfuture currently until the IDE change for the quick fix is merged. If IDE support is there, I'll remove -Xfuture again.
Did you have something else in mind?

@scabug
Copy link
Author

scabug commented Jan 28, 2014

@adriaanm said:
ah right, I guess I forgot the status of this ticket -- sounds good

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@adriaanm said:
Since 2.11.0-RC1 is one week away, pushing all non-blockers without PR to 2.11.1-RC1. Please undo the change if I missed work in progress.

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@soc said:
Why 2.12? The change is already in. The only thing preventing the ticket from being closed is the merge of IDE support, which lacks a test (which I'm working on).

@scabug
Copy link
Author

scabug commented Feb 10, 2014

@adriaanm said:
Sorry, collateral of a bulk change.

@scabug
Copy link
Author

scabug commented Feb 16, 2014

@adriaanm said (edited on Feb 16, 2014 12:15:46 AM UTC):
I don't think scala/scala#2604 was merged, and I can't find a follow up PR.
Rescheduling for 2.11.1-RC1, but feel free to fix my oversight, note the PR that fixes this and mark it as fixed.
Please use the same approach as in #7629, where issues correspond to work items, so that we can close the issue that corresponds to a PR when it's merged. This is the only way we stand a chance at keeping jira in synch with github.

I think these warnings are really important, so I just want to make sure we don't forget about them / include them properly in the release notes.

@scabug
Copy link
Author

scabug commented Feb 16, 2014

@soc said:
Yes, I'm planning to get the IDE support merged with tests as soon as time allows. The testing part has been kind of annoying, so that's was blocking full deprecation currently. I'll look into it.

@scabug
Copy link
Author

scabug commented Aug 5, 2014

@gkossakowski said:
The 2.11.2 is out so I'm rescheduling the issue for 2.11.3.

@scabug
Copy link
Author

scabug commented Nov 4, 2014

@retronym said:
Updating fix-by version to 2.11.5.

@scabug scabug added the lint label Apr 7, 2017
@scabug scabug added this to the Backlog milestone Apr 7, 2017
@som-snytt
Copy link

Subsumed by the linked ticket and also implemented.

@SethTisue SethTisue removed this from the Backlog milestone Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants