typetools / checker-framework

Pluggable type-checking for Java
http://checkerframework.org/
Other
990 stars 347 forks source link

Maven: include location marker in error message #253

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Checkers 1.6.4

If you compile source-code that uses tabs and an error occurs in one of these 
lines then the column number mentioned in the error message which be wrong. 
Checkers treats \t as having a width of one and in most cases this is wrong.

It's probably easier for everyone if you:

1. Omit the column number (only provide a line number).
2. Include a visual representation of the error location as part of the 
message. For example:

AbstractCreateHistory.java:93 incompatible types in argument.
  found   : @Nullable Integer
  required: @NonNull Integer
  text:     columnTypes.add(columnType);
  location:                 *

or

  location: columnTypes.add([*]columnType);

This also makes it easier to understand the error without having to open the 
file/line number in question.

Original issue reported on code.google.com by cow...@bbs.darktech.org on 15 Sep 2013 at 6:20

GoogleCodeExporter commented 8 years ago
The Checker Framework is a plug-in to the OpenJDK Java compiler and we use the 
error reporting mechanism from javac.

We can most definitely improve the text of error messages, so suggestions there 
are very welcome.

The underlying mechanism with column markers however is something we cannot 
change.

Original comment by wdi...@gmail.com on 15 Sep 2013 at 7:37

GoogleCodeExporter commented 8 years ago
Say you leave the column markers as-is, does the error reporting mechanism 
allow you to add the "location" line mentioned above?

Original comment by cow...@bbs.darktech.org on 15 Sep 2013 at 8:29

GoogleCodeExporter commented 8 years ago
I feel your pain:  although it's a standard style guideline not to use tabs in 
source code, sometimes one has to work with legacy code that doesn't follow the 
standards of good style.

Therefore, you might want to take this issue up with Oracle (who maintains 
javac), since reporting a column number in the presence of tab characters is 
indeed misleading!

I'm not sure about the feasibility of the "location line" suggestion, but I 
agree it could be useful and might be worth re-opening this issue for.

Original comment by michael.ernst@gmail.com on 15 Sep 2013 at 8:37

GoogleCodeExporter commented 8 years ago
I'm not quite sure what you mean - a javac error already has a caret ^ pointing 
at the offending location.
Is this maybe different in Maven output?

A command-line javac error for me looks like this:

Department.java:316: error: cannot find symbol
Preconditions.checkNotNullOrEmpty(name, "name");
                          ^
  symbol:   variable Preconditions
  location: class Department

And ignoring space vs. tabs issues, the caret is aligned with the offending 
subexpression.

If you're talking about Maven, maybe there is an option to enable displaying 
the caret?

cu, WMD.

Original comment by wdi...@gmail.com on 15 Sep 2013 at 8:38

GoogleCodeExporter commented 8 years ago
Indeed. The Maven output does not display the caret position. I suspect the 
"option to enable displaying the caret" would have to come from within your own 
Maven plugin.

I'm in the process of checking out the code now but in general many of these 
problems sound like they are supposed to be resolved by the 
checkers-maven-plugin and they are not.

As a side-note, when compiling under Netbeans they find a way to return the 
correct column number in spite of the existence of tabs (for normal code 
compilation). Is there a way for us to ask them how they do it and apply the 
same logic to the checker Maven plugin? (I'm not sure who to contact there)

Original comment by cow...@bbs.darktech.org on 15 Sep 2013 at 9:13

GoogleCodeExporter commented 8 years ago
Ok, I've re-opened the issue and made it specific to Maven.
Jonathan is our Maven expert and he will take care of this and the other issues 
when he's back.

Original comment by wdi...@gmail.com on 15 Sep 2013 at 9:18

GoogleCodeExporter commented 8 years ago
I'm not sure that whatever NetBeans does would translate to a command-line 
tool.  In particular, NetBeans knows the setting of tabs in the user's IDE, but 
the compiler does not know that.  If source code uses tabs (but it shouldn't!), 
then a character after the tab could appear in absolutely any column.  So 
repeating the code -- or having Maven just pass this through from the 
underlying compiler -- sounds like the better approach.

Original comment by michael.ernst@gmail.com on 15 Sep 2013 at 9:21

GoogleCodeExporter commented 8 years ago
The output of the Maven plugin corresponds to that of maven-compiler-plugin, 
which some people may expect (it doesn't affect me personally).

The number that comes from the compiler is really a character offset (not a 
column offset in the strict sense), so there's really no difference in how tabs 
and spaces are handled unless you're building some sort of GUI that doesn't 
already handle that distinction automatically. The caret is offset using the 
same indentation that's present in the source file, so there's no mismatch 
there.

Original comment by atomknig...@gmail.com on 15 Sep 2013 at 9:43

GoogleCodeExporter commented 8 years ago
Thanks for the details, Abraham.  That is helpful.

Abraham's comment suggests that Gili's problem may not be with the Maven plugin 
(since it behaves identically to maven-compiler-plugin), but with whatever GUI 
Gili is using, which is misinterpreting the Maven output.

Original comment by michael.ernst@gmail.com on 15 Sep 2013 at 9:55

GoogleCodeExporter commented 8 years ago
I just ran a test that you might find interesting.

Compiling the same code under Maven and Ant under Netbeans.

Maven output:

JavaApplication.java:[10,8] error: cannot find symbol
1 error

Ant output:

JavaApplication.java:18: error: cannot find symbol
        System.out2.println("test");
              ^
  symbol:   variable out2
  location: class System

I draw three lessons from this:

1. Netbeans doesn't transform the output at all.
2. Maven's output counts \t as a single character (as Abraham said).
3. I find Ant's output to be by far the best:

a) Line numbers without columns.
b) Display offending code.
c) Visual caret position.

Is it possible for us to implement the same thing for the Checker Maven plugin?

Original comment by cow...@bbs.darktech.org on 16 Sep 2013 at 12:21

GoogleCodeExporter commented 8 years ago
Ant just prints the output of javac without modification.

The Maven plugin runs the output through a formatter 
(https://code.google.com/p/checker-framework/source/browse/maven-plugin/src/main
/java/org/checkersplugin/JavacErrorMessagesParser.java) derived from the one 
used by the plexus-compiler-javac compiler (the default Java compiler for 
maven-compiler-plugin: 
https://github.com/sonatype/plexus-compiler/blob/master/plexus-compilers/plexus-
compiler-javac/src/main/java/org/codehaus/plexus/compiler/javac/JavacCompiler.ja
va#L672). I can't find a JIRA ticket explaining why the formatting is there to 
begin with, but I suspect that it has to do with generating consistent output 
across compilers (even across different languages). That being said, it should 
be straightforward to just eliminate the formatting step in the Checker 
Framework Maven plugin to print the javac output verbatim as Ant does (if 
that's what you want to do). Note that if you start using the official Java 8 
compiler via maven-compiler-plugin in the future, you'll be back to square 
one...

Original comment by atomknig...@gmail.com on 16 Sep 2013 at 1:15

GoogleCodeExporter commented 8 years ago
Interesting. So you're saying that maven-checker-plugin isn't meant to be used 
with Java 8 and in the latter's case I would be expected to pass the list of 
processors to maven-compiler-plugin manually?

Original comment by cow...@bbs.darktech.org on 16 Sep 2013 at 2:05

GoogleCodeExporter commented 8 years ago
You will still use maven-checker-plugin to use the Checker Framework, even once 
Java 8 is out. The Checker Framework will not be included in Java 8.

I think what Abraham meant is that, if we improve the error output in 
maven-checker-plugin, you only get that improved with maven-checker-plugin. If 
you switch to any standard Java compiler the Maven output will be back to the 
standard output format.

Original comment by wdi...@gmail.com on 16 Sep 2013 at 2:17

GoogleCodeExporter commented 8 years ago
1. What will be included in Java 8? The ability to annotate types but not the 
specific annotations + processors that are part of Checker Framework?
2. I understand that the standard compiler plugin will still issue the trimmed 
output, whether I use Java7 or Java8 and that's okay. I'm just asking that when 
I use maven-checker-plugin under Java7 or Java8 I get javac's output verbatim.

Original comment by cow...@bbs.darktech.org on 16 Sep 2013 at 2:24

GoogleCodeExporter commented 8 years ago
Re 1: Exactly. The Java source syntax and bytecode storage for type annotations 
will be included in Java 8. Specific uses will not.
Also see
http://types.cs.washington.edu/jsr308/current/jsr308-faq.html#pluggable-type-che
cking-in-java

Re 2: Jonathan will work on the maven-checker-plugin and will decide how to 
best support this use case. Maybe the default behavior will stay the same and 
an additional option will produce verbatim output.

Original comment by wdi...@gmail.com on 16 Sep 2013 at 2:30

GoogleCodeExporter commented 8 years ago
I think that the Checker Framework Maven plugin will/should eventually become 
obsolete. The Checker Framework is effectively a compiler plugin that provides 
annotation processors (i.e. checkers) to be invoked during the compilation 
process. Given that, it should be invoked via the standard 
maven-compiler-plugin executions so as to better integrate with the overall 
build lifecycle. At the moment, the Checker Framework Maven plugin duplicates 
much of the logic in maven-compiler-plugin/plexus-compiler-javac, mostly 
because the Checker Framework requires special configuration to bootstrap a 
special version of javac with the appropriate supporting resources. This works, 
but has some unfortunate consequences (e.g. having to declare a completely new 
plugin execution and disable the existing one). Once Java 8 is released, this 
extra setup should become unnecessary since the compiler APIs included with the 
JDK will already be available in the execution environment.

To put it more concretely, one should eventually be able to invoke the Checker 
Framework using something similar to the following configuration for 
maven-compiler-plugin (this is based on the build infrastructure I've set up at 
my company):

  <pluginManagement>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <configuration>
          <!-- Implied; assuming Java 8 execution environment
          <compilerId>javac</compilerId>
          -->
          <compilerArgs>
            <arg>-Astubs=/path/to/stubs</arg>
            <!-- Other compiler flags if needed -->
          </compilerArgs>
          <annotationProcessors>
            <annotationProcessor>checkers.nullness.NullnessChecker</annotationProcessor>
          </annotationProcessors>
        </configuration>
        <dependencies>
          <dependency>
            <groupId>edu.washington.cs.types.checker</groupId>
            <artifactId>checker-framework</artifactId>
            <!-- Fake version for illustrative purposes -->
            <version>10.0</version>
          </dependency>
          <!-- Other dependencies if needed -->
        </dependencies>
      </plugin>
    </plugins>
  </pluginManagement>

With this configuration, the Checker Framework checkers are executed whenever 
the compiler is normally invoked.

Original comment by atomknig...@gmail.com on 16 Sep 2013 at 3:31

GoogleCodeExporter commented 8 years ago
This makes a lot of sense. Thanks for the clarification.

Original comment by cow...@bbs.darktech.org on 16 Sep 2013 at 3:54

GoogleCodeExporter commented 8 years ago
I am going to work on this now.  I plan on using Werner's suggestion that the 
default behavior remain the same but we add an option for verbatim output. 

Original comment by jbu...@cs.washington.edu on 28 Jan 2014 at 11:37

GoogleCodeExporter commented 8 years ago
This task has been completed.  The useJavacOutput option has been added to the 
plugin and its documentation has been added to the manual.

<useJavacOutput>true</useJavacOutput>

The above option, will cause the compiler output to be logged without extra 
formatting.  That is, the exact Checker Framework compiler output will be 
written to the Maven log.  I have used the name "useJavacOutput" since the 
Checker Framework tries to conform as much as possible to Javac and people 
understand what this type of output looks like.

Original comment by Jonathan...@gmail.com on 29 Jan 2014 at 12:39

GoogleCodeExporter commented 8 years ago

Original comment by Jonathan...@gmail.com on 7 Feb 2014 at 8:55