antlr / antlr3

antlr v3 repository (pulled from p4 with history from //depot/code/antlr/antlr3-main
http://www.antlr.org
249 stars 176 forks source link

@SuppressWarnings("all") on generated Java classes is ineffective #186

Open hakanai opened 8 years ago

hakanai commented 8 years ago

javac apparently ignores @SuppressWarnings("all"). Therefore, this should be replaced by the list of actual warnings ANTLR's generated code triggers, to actually suppress the warnings.

I'm not exactly aware of the complete set of warnings it can create, but here's the common ones from our build.

QueryParserLexer.java:12, Java Compiler (javac), Priority: Normal
no comment

QueryParserLexer.java:74, Java Compiler (javac), Priority: Normal
no @return

Missing Javadoc, basically. I can't tell what the warning category is for this one (it appears to be blank), but perhaps it could be suppressed by generating some "token" (no pun intended) Javadoc.

QueryParserParser.java:138, Java Compiler (javac), Priority: Normal
redundant cast to Object

This one is a bit more perplexing. You can suppress these with SuppressWarnings("cast"). But I get 250 of these "cast" warnings, and it's always to Object. If it's always to Object, why even cast? i.e., for this one, maybe ANTLR could just generate better code.

timeraider4u commented 8 years ago

Which java compiler are you using that is ignoring the "all" statement? Mine (oracle-jdk 1.8.0.121) is quiet fine with it:

harry@harry ~$ echo -e '@SuppressWarnings("all")\npublic class Hello {\n\tpublic static void main (String [] args) {\n\t\tint a=0;\n\t}\n}'  > /tmp/Hello.java && cat /tmp/Hello.java && javac /tmp/Hello.java && echo $?
@SuppressWarnings("all")
public class Hello {
        public static void main (String [] args) {
                int a=0;
        }
}
0
hakanai commented 8 years ago

1.8.0_92 here, but that particular example prints 0 with or without the SuppressWarnings line.

Here's my example. With no suppression:

$ echo -e 'public class Hello {\n\tpublic static void main (String [] args) {\n\t\tString a = (String) "a";\n\t}\n}'  > /tmp/Hello.java && cat /tmp/Hello.java && javac -Xlint:all -Werror /tmp/Hello.java && echo $?
public class Hello {
    public static void main (String [] args) {
        String a = (String) "a";
    }
}
/tmp/Hello.java:3: warning: [cast] redundant cast to String
        String a = (String) "a";
                   ^
error: warnings found and -Werror specified
1 error
1 warning

With @SuppressWarnings("all") there is no difference:

$ echo -e '@SuppressWarnings("all")\npublic class Hello {\n\tpublic static void main (String [] args) {\n\t\tString a = (String) "a";\n\t}\n}'  > /tmp/Hello.java && cat /tmp/Hello.java && javac -Xlint:all -Werror /tmp/Hello.java && echo $?
@SuppressWarnings("all")
public class Hello {
    public static void main (String [] args) {
        String a = (String) "a";
    }
}
/tmp/Hello.java:4: warning: [cast] redundant cast to String
        String a = (String) "a";
                   ^
error: warnings found and -Werror specified
1 error
1 warning

But it does suppress it with @SuppressWarnings("cast"):

$ echo -e '@SuppressWarnings("cast")\npublic class Hello {\n\tpublic static void main (String [] args) {\n\t\tString a = (String) "a";\n\t}\n}'  > /tmp/Hello.java && cat /tmp/Hello.java && javac -Xlint:all -Werror /tmp/Hello.java && echo $?
@SuppressWarnings("cast")
public class Hello {
    public static void main (String [] args) {
        String a = (String) "a";
    }
}
0
hakanai commented 8 years ago

Also tried on Java 9-ea, since that happened to be sitting around on my computer, and same result on that version.

timeraider4u commented 8 years ago

Yes, with the -Xlint:all command line option its now the same for me. Seems I am not using it anywhere...

hakanai commented 8 years ago

This makes some sense then. I guess "cast" type warnings are disabled by default.

timeraider4u commented 8 years ago

Ah okay, according to http://docs.oracle.com/javase/7/docs/technotes/tools/windows/javac.html#nonstandard right, -Xlint with some optional argument is enabling the according warnings.

Interestingly the following is working:

~ $ echo -e '@SuppressWarnings("cast")\npublic class Hello {\n\tpublic static void main (String [] args) {\n\t\tString a = (String) "a";\n\t}\n}' > /tmp/Hello.java && cat /tmp/Hello.java && javac -Xlint:all -Werror /tmp/Hello.java && echo $?
@SuppressWarnings("cast")
public class Hello {
        public static void main (String [] args) {
                String a = (String) "a";
        }
}
0

while this example fails:

echo -e '@SuppressWarnings("all")\npublic class Hello {\n\tpublic static void main (String [] args) {\n\t\tString a = (String) "a";\n\t}\n}' > /tmp/Hello.java && cat /tmp/Hello.java && javac -Xlint:all -Werror /tmp/Hello.java && echo $?
@SuppressWarnings("all")
public class Hello {
        public static void main (String [] args) {
                String a = (String) "a";
        }
}
/tmp/Hello.java:4: warning: [cast] redundant cast to String
                String a = (String) "a";
                           ^
error: warnings found and -Werror specified
1 error
1 warning

I would consider this a bug in the javac-compiler itself but maybe I am missing something

hakanai commented 8 years ago

I think it's just that "all" is not a class of warning, and not handled specially by the compiler either, so it doesn't suppress anything.

The confusing thing is that IDEs do treat it as a special value.

timeraider4u commented 8 years ago

According to the Java specification at http://docs.oracle.com/javase/specs/jls/se7/html/jls-9.html#jls-9.6.3.5 its up to Java implementations if "all" is a valid value.

E.g., for Eclipse the suppressing string "all" is well defined - see http://help.eclipse.org/kepler/index.jsp?topic=%2Forg.eclipse.jdt.doc.user%2Ftasks%2Ftask-suppress_warnings.htm

hakanai commented 8 years ago

Yeah. I also remember that previous versions of either Eclipse or IDEA didn't recognise "all", but did recognise "ALL", so in some places in our code we ended up with @SuppressWarnings({"all", "ALL"}).

So anyway, it seems that we'd want "all" and "cast", even though it seems counterintuitive...

Or I guess the warnings could be fixed. In my case, at least, every single cast warning is about a cast to Object.