Less than ideal error message when incorrectly trying to use optional parenthesis

Details

  • Type: Sub-task Sub-task
  • Status: Open Open
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: 1.0-JSR-1
  • Fix Version/s: None
  • Component/s: parser-antlr
  • Labels:
    None
  • Environment:
    GNU/Linux 2.6.8, JDK 1.5.0-b64, Groovy CVS at 2005.04.15

Description

The script:

adob = new ArrayList ( )
adob.add "hello"
println adob.get 0

generates the following error report. The report is seemingly incorrect (why should it be expecting EOF) and say nothing about the real problem which is that the ( was not found. The problem is in the error reporting not in the grammar.

Caught: General error during parsing: expecting EOF, found '0'

/home/users/russel/Progs/Groovy/./topLevel.groovy:5:18: expecting EOF, found '0'
at antlr.Parser.match(Parser.java:211)
at org.codehaus.groovy.antlr.parser.GroovyRecognizer.compilationUnit(GroovyRecognizer.java:675)
at org.codehaus.groovy.antlr.AntlrParserPlugin.parseCST(AntlrParserPlugin.java:79)
at org.codehaus.groovy.control.SourceUnit.parse(SourceUnit.java:297)
at org.codehaus.groovy.control.CompilationUnit$3.call(CompilationUnit.java:352)
at org.codehaus.groovy.control.CompilationUnit.applyToSourceUnits(CompilationUnit.java:594)
at org.codehaus.groovy.control.CompilationUnit.parse(CompilationUnit.java:343)
at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:313)
at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:235)
at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:204)
at groovy.lang.GroovyClassLoader.parseClass(GroovyClassLoader.java:154)
at groovy.lang.GroovyShell$1.run(GroovyShell.java:261)
at java.security.AccessController.doPrivileged(Native Method)
at groovy.lang.GroovyShell.run(GroovyShell.java:259)
at groovy.lang.GroovyShell.run(GroovyShell.java:202)
at groovy.ui.GroovyMain.processOnce(GroovyMain.java:397)
at groovy.ui.GroovyMain.run(GroovyMain.java:233)
at groovy.ui.GroovyMain.process(GroovyMain.java:219)
at groovy.ui.GroovyMain.main(GroovyMain.java:123)
at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:92)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:585)
at org.codehaus.classworlds.Launcher.launchEnhanced(Launcher.java:315)
at org.codehaus.classworlds.Launcher.launch(Launcher.java:255)
at org.codehaus.classworlds.Launcher.mainWithExitCode(Launcher.java:429)
at org.codehaus.classworlds.Launcher.main(Launcher.java:375)

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Russel Winder added a comment -

This script, well actually this one:

def adob = new ArrayList ( )
adob.add "hello"
println adob.get 0

now generates the error message:

./test.groovy: 5: expecting EOF, found '0' @ line 5, column 18.
println adob.get 0
^

1 Error

which is both good and bad. It is good as the information content and presentation is just so, so, so much better (thanks to all those whose efforts went into that change). However, it is bad because the actual error message has to be wrong. Surely the parse should be expecting ( not EOF?

Putting:

def adob = new ArrayList ( )
adob.add "hello"
println adob.get ( 0 )

gives the expected output, whereas:

def adob = new ArrayList ( )
adob.add "hello"
println adob.get

i.e. replacing the 0 with an EOF gives the output:

Caught: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.String

which is not exactly unreasonable. But wrong

Show
Russel Winder added a comment - This script, well actually this one: def adob = new ArrayList ( ) adob.add "hello" println adob.get 0 now generates the error message: ./test.groovy: 5: expecting EOF, found '0' @ line 5, column 18. println adob.get 0 ^ 1 Error which is both good and bad. It is good as the information content and presentation is just so, so, so much better (thanks to all those whose efforts went into that change). However, it is bad because the actual error message has to be wrong. Surely the parse should be expecting ( not EOF? Putting: def adob = new ArrayList ( ) adob.add "hello" println adob.get ( 0 ) gives the expected output, whereas: def adob = new ArrayList ( ) adob.add "hello" println adob.get i.e. replacing the 0 with an EOF gives the output: Caught: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.String which is not exactly unreasonable. But wrong
Hide
Permalink
Paul King added a comment -

Moreover, this code:

def adob = new ArrayList ( )
adob.add 3
println adob.get

Returns the message:

Caught: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer

And this code:

def adob = new ArrayList ( )
println adob.get

Returns no error, just:

[]
Show
Paul King added a comment - Moreover, this code: 
def adob = new ArrayList ( )
adob.add 3
println adob.get
Returns the message: 
Caught: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer
And this code: 
def adob = new ArrayList ( )
println adob.get
Returns no error, just: 
[]
Hide
Permalink
Guillaume Laforge added a comment -

The message could probably be improved if it's possible.

Show
Guillaume Laforge added a comment - The message could probably be improved if it's possible.
Hide
Permalink
Paul King added a comment -

There are two places where we would need to improve the error message:

  • the EOF message when 'get 0' is in the source
  • the potentially unexpected message when get is performed on the items in the list

One way to improve the error message for the second of these cases would be to change MetaClassImpl from:

} else if (object instanceof Collection) {
                return DefaultGroovyMethods.getAt((Collection) object, name);

to:

} else if (object instanceof Collection) {
                try {
                    return DefaultGroovyMethods.getAt((Collection) object, name);
                } catch (MissingPropertyExceptionNoStack mpe) {
                    RuntimeException cause = new MissingPropertyException(mpe.getProperty(), mpe.getType());
                    throw new MissingPropertyException(name, object.getClass(), cause);
                }

this would then have an error message like:

groovy.lang.MissingPropertyException: No such property: get for class: java.util.ArrayList. Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer

but I think it might be better to have a separate exception: ListPropertyEvaluationException or similar, then the error message could be something like:

groovy.lang.ListPropertyEvaluationException: Unable to evaluate properties for: java.util.ArrayList. Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer

I guess it would be best to have a term that wasn't list as it could be any Collection and I guess something that worked for Object[] would be best.

Show
Paul King added a comment - There are two places where we would need to improve the error message:
  • the EOF message when 'get 0' is in the source
  • the potentially unexpected message when get is performed on the items in the list
One way to improve the error message for the second of these cases would be to change MetaClassImpl from: 
} else if (object instanceof Collection) {
                return DefaultGroovyMethods.getAt((Collection) object, name);
to: 
} else if (object instanceof Collection) {
                try {
                    return DefaultGroovyMethods.getAt((Collection) object, name);
                } catch (MissingPropertyExceptionNoStack mpe) {
                    RuntimeException cause = new MissingPropertyException(mpe.getProperty(), mpe.getType());
                    throw new MissingPropertyException(name, object.getClass(), cause);
                }
this would then have an error message like: 
groovy.lang.MissingPropertyException: No such property: get for class: java.util.ArrayList. Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer
but I think it might be better to have a separate exception: ListPropertyEvaluationException or similar, then the error message could be something like: 
groovy.lang.ListPropertyEvaluationException: Unable to evaluate properties for: java.util.ArrayList. Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.Integer
I guess it would be best to have a term that wasn't list as it could be any Collection and I guess something that worked for Object[] would be best.
Hide
Permalink
Paul King added a comment -

The EOF issue still exists but the other case now has this error message:

Caught: groovy.lang.MissingPropertyException: Exception evaluating property 'get' for java.util.ArrayList, Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.String
Show
Paul King added a comment - The EOF issue still exists but the other case now has this error message: 
Caught: groovy.lang.MissingPropertyException: Exception evaluating property 'get' for java.util.ArrayList, Reason: groovy.lang.MissingPropertyException: No such property: get for class: java.lang.String
Hide
Permalink
Paul King added a comment -

Updated title to better reflect current status of issue

Show
Paul King added a comment - Updated title to better reflect current status of issue

People

  • Assignee:
    Unassigned
    Reporter:
    Russel Winder
Vote (1)
Watch (4)

Dates

  • Created:
    Updated: