groovy

groovyc should emit errors when @Override is not respected

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: Compiler
  • Labels:
    None
  • Number of attachments :
    0

Description

The following compiles without warning or error:

class OverrideAnnotatationTest {
    @Override
    def notOverriden() {}
}

Issue Links

is depended upon by

Bug - A problem which impairs or prevents the functions of the product. GRECLIPSE-1428 Groovy editor does not respect @Override

  • Major - Major loss of function.
  • Open - The issue is open and ready for the assignee to start work on it.
is related to

Improvement - An improvement or enhancement to an existing feature or task. GROOVY-5011 @Override should be allowed (and verified) on properties that implement an interface

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Open - The issue is open and ready for the assignee to start work on it.
relates to

Bug - A problem which impairs or prevents the functions of the product. GRECLIPSE-1454 no Eclipse compile error for @Override on methods with signatures that don't match that of the parent class

  • Major - Major loss of function.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Paul King made changes -
Field Original Value New Value
Link This issue is related to GROOVY-5011 [ GROOVY-5011 ]
Hide
Permalink
Paul King added a comment - - edited

Linking to existing issue wrt properties

Show
Paul King added a comment - - edited Linking to existing issue wrt properties
Hide
Permalink
Paul King added a comment - - edited

The other question is whether to support a "Java 5" type mode where @Override wouldn't be allowed for interfaces but just super classes. I think we should avoid unless someone has a compelling reason.

Show
Paul King added a comment - - edited The other question is whether to support a "Java 5" type mode where @Override wouldn't be allowed for interfaces but just super classes. I think we should avoid unless someone has a compelling reason.
Hide
Permalink
Paul King added a comment -

Proposed fix:
https://github.com/paulk-asert/groovy-core/commit/b7e9cd648cd13d6f88803ed556b49ddc4e18d3da

Show
Paul King added a comment - Proposed fix: https://github.com/paulk-asert/groovy-core/commit/b7e9cd648cd13d6f88803ed556b49ddc4e18d3da
Hide
Permalink
blackdrag blackdrag added a comment -

I don't think we need a Java5 mode. As for the patch... I think covariant returns should be ok in ExtendedVerifier. What I wonder about are generics. I think it will not work like this form them

Show
blackdrag blackdrag added a comment - I don't think we need a Java5 mode. As for the patch... I think covariant returns should be ok in ExtendedVerifier. What I wonder about are generics. I think it will not work like this form them
Andrew Eisenberg made changes -
Link This issue is depended upon by GRECLIPSE-1428 [ GRECLIPSE-1428 ]
Andrew Eisenberg made changes -
Link This issue relates to GRECLIPSE-1454 [ GRECLIPSE-1454 ]
Hide
Permalink
Thomas Weissinger added a comment -

What really is troublesome about this is, let's say you changed the base class method declaration without changing the corresponding subclass (or subclasses). You will have no idea what method has the problem. You also can't determine what line in the Groovy file had the problem. You end up having to open the Java stub to work your way back to determine what Groovy function is wrong.

Here's what you may see:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.3.2:compile (default-compile) on project bar: Compilation failure: Compilation failure:
[ERROR] /foo/bar/target/generated-sources/groovy-stubs/main/com/trw/FooBar.java:[28,0] method does not override or implement a method from a supertype

Show
Thomas Weissinger added a comment - What really is troublesome about this is, let's say you changed the base class method declaration without changing the corresponding subclass (or subclasses). You will have no idea what method has the problem. You also can't determine what line in the Groovy file had the problem. You end up having to open the Java stub to work your way back to determine what Groovy function is wrong. Here's what you may see: [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.3.2:compile (default-compile) on project bar: Compilation failure: Compilation failure: [ERROR] /foo/bar/target/generated-sources/groovy-stubs/main/com/trw/FooBar.java: [28,0] method does not override or implement a method from a supertype
Hide
Permalink
Henri Pihkala added a comment -

Any progress on this?

Show
Henri Pihkala added a comment - Any progress on this?
Hide
Permalink
blackdrag blackdrag added a comment -

@Henri does it solve the issue for you?

@Paul I think we can maybe just add this. There may be issues, but we can work them out later

Show
blackdrag blackdrag added a comment - @Henri does it solve the issue for you? @Paul I think we can maybe just add this. There may be issues, but we can work them out later
Hide
Permalink
Henri Pihkala added a comment -

@blackdrag This feature would make detecting refactoring issues much easier, plus one might expect Java-like behavior in this case. I don't have the means to test the patch but I assume it works as intended.

Show
Henri Pihkala added a comment - @blackdrag This feature would make detecting refactoring issues much easier, plus one might expect Java-like behavior in this case. I don't have the means to test the patch but I assume it works as intended.
Hide
Permalink
Pascal Schumacher added a comment -

Shouldn't we add Pauls fix?

Show
Pascal Schumacher added a comment - Shouldn't we add Pauls fix?
Pascal Schumacher made changes -
Component/s Compiler [ 13529 ]
Hide
Permalink
blackdrag blackdrag added a comment -

I assign it to Paul

Show
blackdrag blackdrag added a comment - I assign it to Paul
blackdrag blackdrag made changes -
Assignee Paul King [ paulk ]
Hide
Permalink
Paul King added a comment -

The other point that perhaps hasn't been clarified before is how to deal with optional parameters:

@Override
def method(int firstArg, String secondArg = 'some default') {
    ...
}

Under the covers Groovy will expand this roughly into:

@Override
def method(int firstArg) {
    method(firstArg, 'some default')
}
@Override
def method(int firstArg, String secondArg) {
    ...
}

The intention being that @Override is added to both methods. Currently there is no way to limit the annotation to occur on just one of the methods and I don't think it is in the scope of this issue to consider such cases.

Show
Paul King added a comment - The other point that perhaps hasn't been clarified before is how to deal with optional parameters: @Override def method( int firstArg, String secondArg = 'some default ') { ... } Under the covers Groovy will expand this roughly into: @Override def method( int firstArg) { method(firstArg, 'some default ') } @Override def method( int firstArg, String secondArg) { ... } The intention being that @Override is added to both methods. Currently there is no way to limit the annotation to occur on just one of the methods and I don't think it is in the scope of this issue to consider such cases.

People

  • Assignee:
    Paul King
    Reporter:
    Bob Tiernay
Vote (2)
Watch (6)

Dates

  • Created:
    Updated: