Handout P8

Contents:

Introduction

It is better to detect a bug at compile time than at run time. In this problem set, you will make your rational polynomial and Husky Maps code more robust, with the help of a type system that is stronger than Java's built-in type system.

You have had experience with the errors that the Java compiler prevents, and with the errors that you discover through testing. This problem set will give you exposure to more powerful type systems that prevent event more errors at compile-time. You will come to appreciate the strengths and weaknesses of these type systems, and will strengthen your understanding of type systems in general. And, you will be exposed to a cutting-edge research tool and will get an early view of (one aspect of) the future evolution of the Java language.

Before you start, read the entire problem set.

Before you start, also read the Checker Framework Manual, chapters 1, 2, 3, and 16, and skim chapters 15, 20, and 21. Do not stint on this step; it will save you time in the long run. Feel free to ask questions of the course staff.

As usual, the source code that you need can be found by updating your cse331 directory from SVN.

Building with Ant

Setup

Before using Ant on Linux (including attu) for this problem set, you must set the CHECKERS environment variable. One way to do so is to add the following line to your ~/.bashrc file:

  export CHECKERS="/cse/courses/cse331/checkers"

After modifying your .bashrc file, you will need to re-login for the changes to take effect.

Additional Setup for Eclipse on Windows

In order to use the build file from Eclipse on Windows, you must set the CHECKERS and PATH variables within Eclipse:

  1. Right click on ps8/build.xml and select "External Tools Configurations" from the context menu
  2. Click on the "Environment" tab
  3. Add a variable named CHECKERS with the value O:\cse\courses\cse331\checkers
  4. Add a variable named PATH with the value C:\Program Files\Java\jdk1.6.0_17 (or wherever the JDK is installed on your computer)
  5. Check that "Append environment to native environment" is selected

Ant Targets

Once you have set environment variables, you can run the Nullness Checker on the different parts of the problem set with the following commands:

The validate target performs build.poly and build.husky on a clean checkout from your repository.

Warnings as Errors

For this problem set, we've enabled all Java warnings with the -Xlint flag and have told the compiler to treat all warnings as errors. Therefore, in order to complete this problem set, you will need to eliminate all warnings from your projects.

Eliminate null pointer errors (30 points)

Your goal in this problem set is to eliminate all possible null pointer errors from your CSE 331 code. We wish to have a compile-time guarantee that your code can throw no NullPointerException. You will obtain the guarantee by running the Nullness Checker on your code.

In order for your code to pass the Nullness Checker, you will annotate your program with /*@Nullable*/ annotations to indicate which variables may possibly be null. You may also use other annotations supported by the Nullness Checker. (You probably won't need to write any /*@NonNull*/ annotations, because that is the default.)

You don't need to annotate your testing code (that is, anything in test/ directories). We have annotated the ps6/tigerdb package for you and have pushed the annotated version to your repository.

You need to add just the right number of annotations. The final annotations must be consistent with one another &mdash for example, a possibly-null value must never be assigned to a non-null variable. The annotations must also be consistent with your code: any dereference such as x.field or x.method() may only be performed on a non-null variable. If you add too many or too few annotations, then one or the other of these properties will be violated, and the Nullness Checker will issue an error.

You don't need to add any import statements for classes like checkers.nullness.quals.Nullable, because the Ant build file is configured to have the compiler automagically insert import checkers.nullness.quals.* for you. (See Section 16.3.2 in the Checker Framework documentation).

Examples

See the files NullnessExample.java and NullnessExampleWithWarnings.java in your repository. Verify that compiling the latter issues warnings. Modify it to eliminate the warnings (you can use NullnessExample.java as a cheat sheet) and verify that compiling the corrected version issues no warnings. Now, you are ready to do the same for your own code.

You may notice that the provided examples generate compiler errors in Eclipse but will compile properly with the Ant build file. This section describes the cause of the errors and how to fix them.

The first error you may notice is that "The import checkers cannot be resolved". This error occurs because Eclipse does not know where to look for the checker package, which is located at /cse/courses/cse331/checkers/checkers.jar. To eliminate this error, you should just remove the checker import statements from both example files. The Ant build file is configured to automatically perform these imports for you.

Next, you may notice errors that "NonNull cannot be resolved to a type". These errors occur because the Eclipse compiler is not importing the package checkers.nullness.quals.* which contains the annotation definitions. To eliminate the error, you should enclose the nullness annotations in comments -- i.e., /*@NonNull*/.

Finally, you may notice that the nullness annotations on generic parameters were enclosed in comments while those on local variables types were not. This is because annotations on functions and variable types have been supported since Java 1.5, but annotations on generic parameters will not be supported until Java 1.7. The examples were designed to be backwards compatible.

Suppressing warnings

Your goal is to obtain a compile-time guarantee of correct use of null. Therefore, you are strongly discouraged from suppressing warnings.

If you suppress 5 or more warnings, you are doing something seriously wrong and should talk to a staff member. It is perfectly reasonable to complete the assignment without suppressing any warnings, and that should be your goal.

For each suppressed warning, you should write a single sentence explaining why the @SuppressWarnings annotation or assert statement is correct. In other words, explain why adding the annotation/assertion does not compromise the guarantee that your program never throws a NullPointerException. The explanation should be a Java comment immediately before the annotation/assertion.

Rather than suppressing a warning, it is usually better to refactor your code so that the checker issues no warnings. That usually improves the design: if the correctness of your code is so obvious that it is apparent to the checker, then it will also be more apparent to other programmers!

If you have to suppress a warning, try to suppress it as early as possible, such as at at earlier line of a method rather than a later one, if you have a choice. Placing the suppression earlier generally leads to clearer code.

The Checker Framework Manual states a number of ways to suppress warnings. You should only use @SuppressWarnings annotation or the assert statement, preferring to use the annotation when possible. Each such annotation should be on a single variable declaration (not on an entire method or class). For example:

// No @SuppressWarnings("nullness") annotation here on the class!
class MyClass {

  // No @SuppressWarnings("nullness") annotation here on the method!
  void myMethod() {
    ...
    @SuppressWarnings("nullness")
    /*@NonNull*/ Object myVar = expression;
    ...
  }

  ...
}

Do not work around the type-checker

Do not work around the type-checker. One example would be replacing code such as

  x.method();

with

  if (x == null)
    throw new Error("x is null!");
  x.method();

That replacement may quiet the compiler warnings, but it does does no good in terms of guaranteeing that your program does not crash: the original version crashes if x is null, and so does the modified version. Instead, figure out how to prove that x can never be null. (And, that Error is not documented in the method's specification, so throwing it violates the spec.)

A reasonable rule of thumb is: don't add a test that cannot fail at run time, just to eliminate type-checker warnings. Any programmer reading a test will assume that it is possible for it to fail, so the added code degrades the readability of your code.

Grading criteria

After you have refactored and annotated your code, it should still pass all of your tests (and ideally the staff tests as well).

Part of your grade will depend on the number of times you suppress warnings, and whether the explanations are clear enough that the graders can immediately see that they are correct.

Assessment of pluggable type-checking (12 points)

Like any tool, pluggable type-checking has its strengths and weaknesses, and should only be used in appropriate ways and situations.

Please answer the following questions in a file named assessment.txt in your answers/ directory. There are no wrong or right answers, but we expect your answers to be thoughtful.

  1. Did you discover null pointer errors in your code? If so, describe at least one of them. Why did testing fail to reveal this error?
  2. Describe the largest or most difficult change that you made, as a result of running the checker, that improved your design or your code. How long did it take you to make the change? Would it have been easy to choose a nullness-correct design in the first place, if you had been thinking about nullness-correctness from the beginning? Be concrete — name the class and method(s). You may paste code but are not required to do so.
  3. Describe the largest or most difficult change that you made, as a result of running the checker, that degraded your design or your code. How long did it take you to make the change? Why did you have to make this change? As in the previous part, be concrete.
  4. Describe the most frequent change that you made — where you had to do the same thing repeatedly in different parts of your code. (In this entire question, “change” means a change to code, not merely adding an annotation.) Give a single example, by pasting from your source code. How often did you have to make such a change?
    Or, state that no such pattern existed.
  5. What missing feature or behavioral difference, either in the checker or its set of annotations, would have made your task easier? (We already know that fixing the Eclipse plug-in would be nice; mention something else that you noticed.)
  6. Suppose your goal was to improve your code. Is obtaining a static guarantee of no null pointer errors the best use of time? If not, what would be? For example, do you think you would have obtained better code by spending the same amount of time on other tasks such as writing tests, reasoning about the code, rewriting some of it, writing more assertions, playing with the software to see if the directions it produces are suspicious, etc.?
  7. Briefly describe a defect (bug) that you have encountered in the class that caused you difficulty. It should not be nullness-related. Would this defect be amenable to discovery via type-checking? (Some are, and some are not.) Explain why or why not.

Collaboration (1 point)

Please answer the following questions in a file named collaboration.txt in your answers/ directory.

The standard collaboration policy applies to this problem set.

State whether or not you collaborated with other students. If you did collaborate with other students, put their names and a brief description of how you collaborated.

Reflection (1 point)

Please answer the following questions in a file named reflection.txt in your answers/ directory.

  1. How many hours did you spend on each problem of this problem set? (Only include time when you are completely focused on CSE 331. For example, if you worked an hour in your noisy dorm lounge, or you were periodically reading email or IMs, don't count that hour.)
  2. In retrospect, what could you have done better to reduce the time you spent solving this problem set?
  3. What could the CSE 331 staff have done better to improve your learning experience in this problem set?
  4. What do you know now that you wish you had known before beginning the problem set?

What to turn in

The following files should be checked into SVN before you run ant validate:

There are no new code files to submit for this problem set. Your work will be reflected in the code from previous problem sets, which you will have annotated with /*@Nullable*/ and other annotations supported by the Nullness Checker. The Nullness Checker should issue no warnings when it is run on your code, and ant validate will check this.

Working at home

As with all problem sets, you may work at home or on an instructional workstation such as attu.

Working at home takes a little bit more effort with this problem set, because you will need to install the Checker Framework. The installation is a simple 3-step process that is documented in the Checker Framework Manual.

After installing the Checker Framework, you will need to modify your environment variables as previously described.

Do not attempt to use the Eclipse plug-in for the Checker Framework (it suffers from some bugs you are likely to encounter); instead, use the Ant build file.

Hints

Many more hints appear in the Checker Framework Manual. One example is Section 2.4.4, How to get started annotating legacy code.

If you're still having trouble, the forums are a good place to look for help.

Write type annotations in comments &mdash for example, always use /*@Nullable*/, not @Nullable. You do not need to, and should not, write declaration annotations, such as @SuppressWarnings("nullness"), in comments.

In general, you should write few or no annotations within method bodies — most annotations will be on signatures, and you should not write annotations on method bodies only when absolutely necessary.

You are usually better off writing as many annotations as possible (on signatures!) first, and then run the checker when you think you are done. That is more efficient than starting out by running the checker on unannotated code, trying to fix the warnings, and iterating.

Where to write annotations

A good way to proceed is to express your rep invariant as annotations. That will help you ensure that your code does not violate the rep invariant, and a strong rep invariant will make it easier to ensure that no null pointer exception occurs. For example, suppose that your Graph representation is

    // maps from each node to all of its children
    Map<Node, Set<Node>> edges;

and your rep invariant is:

Because /*@NonNull*/ is the default, the following annotation can express this entire invariant! Then, the type-checker will warn if your code violates the rep invariant.

    Map<Node, Set</*@KeyFor("edges")*/ Node>> edges;

Likewise, you should express as many other invariants of your implementations as annotations. Whenever there is an invariant that you cannot express as an annotation, that is a good indication that you will probably need to suppress some false positive warnings.

Another trick is to search your source code for the string “null” (in documentation or source code), and to write /*@Nullable*/ annotations on signatures accordingly.

You do not have to write annotations on the JDK. If you find that there is a missing or incorrect annotation on the JDK, then just suppress the related warning and inform the course staff.

Acceptable suppressed warnings

This section is a subset of acceptable places to suppress a warning, due to a bug in the Nullness Checker. (Your comment can state that the suppression is permitted per the ”acceptable suppressed warnings“ section of the problem set.) Other acceptable places exist, but you should strive to suppress as few warnings as possible, and your first inclination should always be to find a way to avoid the suppression (without just turning it into some other exception besides a NullPointerException, of course).

Old acceptable suppressed warnings

These bugs in the Nullness Checker have been fixed, so these warning suppressions are no longer necessary. The fix has been installed on attu, but if you are working at home, you will need to check the date on the Checker Framework distribution to see whether it contains the fix. If not, then you may need to suppress the warning while working at home, then remove it when you finally turn in your problem set.

Searching for suppressed warnings

You can use the following Linux command to see where you've suppressed nullness warnings for this problem set:

  find . -name '*.java' -o -name test -prune -o -name tigerdb -prune | xargs grep -C 3 -n nullness

The -C 3 argument tells grep to provide 3 lines of context around each line that contains the string "nullness".

You can also use Subversion to see all the changes you have made to your code since starting the problem set; that may be useful when reviewing your changes.

Refactoring Hints

You will need to refactor the paint method in ps1/PolyGraph.java to accommodate the xValBuffer and yValBuffer variables; the nullness checker is not strong enough to reason about the control flow that is dependent on calcFrame.stack.size().

At lines 137 and 144 of ps6/StreetSegIterator.java, you should just remove the casts to (GeoChain) and (File).

More ways to check your code

If you find the problem set too easy (if your code is well-written, you will finish it very fast!), you could try using the IGJ mutability checker (which was demoed in lecture) or FindBugs.

There isn't any credit for this (it won't affect the curve for the class as a whole), but if you try this, we will take it into account after assigning all grades, but before recording them.

Errata

None yet!

Q & A

Q: When I write an annotation, Eclipse cannot find my classes, saying "MyClass cannot be resolved as type".

Go to Project -> Clean. In the "Clean" dialog make sure that "Start a build immediately" is checked and click "OK". If the "Start a build immediately" checkbox in the "Clean" dialog is missing, then uncheck Project -> Build Automatically, and then try again.

Q: I get an error related to JPanel.setLayout.

Run svn update to get the latest version of the code.

Q: What does the type @NonNull Object @Nullable [] mean?

@NonNull Object @Nullable [] is a possibly-null array of non-null objects. Note that even though the type starts with “@NonNull”, that applies to the element type Object. The annotation @Nullable applies to the array ([].

Similarly, @Nullable Object @NonNull [] is a non-null array of possibly-null objects.

Q: I get compiler errors for the staff-supplied code ps6/StreetSegReader.java

See the hint above.

Q: I get a warning "No processor claimed any of these annotations"

This warning is innocuous. You can ignore it.