aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStephan Herrmann2012-05-28 02:36:17 (EDT)
committerDeepak Azad2012-05-28 02:36:17 (EDT)
commitf34642c5440cef73e31a0cedcef117ef8a599423 (patch)
tree3540bdbb34de30e748181ae49d007a1423c9ce25
parent40cf5816329b8239188c181983a3c816cc8804c3 (diff)
downloadeclipse.platform.common-f34642c5440cef73e31a0cedcef117ef8a599423.zip
eclipse.platform.common-f34642c5440cef73e31a0cedcef117ef8a599423.tar.gz
eclipse.platform.common-f34642c5440cef73e31a0cedcef117ef8a599423.tar.bz2
Bug 380074: Add doc for 'Detecting problems in Java code'
-rw-r--r--bundles/org.eclipse.jdt.doc.user/reference/preferences/java/compiler/ref-preferences-errors-warnings.htm6
-rw-r--r--bundles/org.eclipse.jdt.doc.user/tasks/task-improve_code_quality.htm34
-rw-r--r--bundles/org.eclipse.jdt.doc.user/tasks/task-using_null_annotations.htm385
-rw-r--r--bundles/org.eclipse.jdt.doc.user/topics_Tasks.xml3
4 files changed, 425 insertions, 3 deletions
diff --git a/bundles/org.eclipse.jdt.doc.user/reference/preferences/java/compiler/ref-preferences-errors-warnings.htm b/bundles/org.eclipse.jdt.doc.user/reference/preferences/java/compiler/ref-preferences-errors-warnings.htm
index 2b67681..366ebed 100644
--- a/bundles/org.eclipse.jdt.doc.user/reference/preferences/java/compiler/ref-preferences-errors-warnings.htm
+++ b/bundles/org.eclipse.jdt.doc.user/reference/preferences/java/compiler/ref-preferences-errors-warnings.htm
@@ -809,7 +809,7 @@ use raw types in the first place.</p>
</tr>
</table>
-<h3>Null analysis</h3>
+<h3 id="null_analysis">Null analysis</h3>
<table border="1" cellspacing="0" cellpadding="5" width="100%" summary="Annotations section">
<tr>
@@ -871,7 +871,7 @@ use raw types in the first place.</p>
<p>Off</p>
</td>
</tr>
- <tr>
+ <tr id="null_spec_violation">
<td valign="top" style="padding-left: 2em;">
<p>Violation of null specification</p>
</td>
@@ -955,7 +955,7 @@ use raw types in the first place.</p>
<p>Ignore</p>
</td>
</tr>
- <tr>
+ <tr id="null_annotation_names">
<td valign="top" style="padding-left: 2em;">
<p>Use default annotations for null specifications</p>
</td>
diff --git a/bundles/org.eclipse.jdt.doc.user/tasks/task-improve_code_quality.htm b/bundles/org.eclipse.jdt.doc.user/tasks/task-improve_code_quality.htm
new file mode 100644
index 0000000..5e62e16
--- /dev/null
+++ b/bundles/org.eclipse.jdt.doc.user/tasks/task-improve_code_quality.htm
@@ -0,0 +1,34 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html lang="en">
+<head>
+<meta name="copyright" content="Copyright (c) GK Software AG and others 2012. This page is made available under license. For full details see the LEGAL in the documentation book that contains this page." >
+<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
+<meta http-equiv="Content-Style-Type" content="text/css">
+<link rel="stylesheet" href="../book.css" charset="ISO-8859-1" type="text/css">
+<title>Improving Java code quality</title>
+</head>
+<body>
+<h1> Improving Java code quality </h1>
+<p>
+The Eclipse Java compiler performs more checks and analyses than are mandated by the Java Language Specification.
+This is done in order to help you to improve the quality of your Java code.
+Since different users have different views of which warnings are interesting, this behavior is highly configurable.
+See <a href="../reference/preferences/java/compiler/ref-preferences-errors-warnings.htm">Java Compile Errors/Warnings Preferences</a>
+for available options.
+</p>
+<p>
+Some warnings and errors should be obvious and generally valid for everybody.
+In some cases you may want to configure the compiler to match your code style
+and also your quality goals.
+Some analyses are most helpful if you to some degree adjust your code style
+to make it better analyzable by the compiler.
+</p>
+<p>
+The pages listed below give some background on certain analyses and hints
+on how to make the best use of them:
+</p>
+<ul>
+<li><a href="task-using_null_annotations.htm">Using null annotations</a></li>
+</ul>
+</body>
+</html>
diff --git a/bundles/org.eclipse.jdt.doc.user/tasks/task-using_null_annotations.htm b/bundles/org.eclipse.jdt.doc.user/tasks/task-using_null_annotations.htm
new file mode 100644
index 0000000..d41ad39
--- /dev/null
+++ b/bundles/org.eclipse.jdt.doc.user/tasks/task-using_null_annotations.htm
@@ -0,0 +1,385 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html lang="en">
+<head>
+<meta name="copyright" content="Copyright (c) GK Software AG and others 2012. This page is made available under license. For full details see the LEGAL in the documentation book that contains this page." >
+<meta http-equiv="Content-Type" content="text/html; charset=ISO-8859-1">
+<meta http-equiv="Content-Style-Type" content="text/css">
+<link rel="stylesheet" href="../book.css" charset="ISO-8859-1" type="text/css">
+<title>Using null annotations</title>
+</head>
+<body>
+<h1> Using null annotations </h1>
+<p><code>NullPointerException</code> is one of the most common causes for failure of Java programs.
+In the simplest cases the compiler can directly warn you when it sees code like this:
+</p>
+<pre> Object o = null;
+ String s = o.toString();
+</pre>
+<p>
+With branching / looping and throwing exceptions quite sophisticated <strong>flow analysis</strong>
+becomes necessary in order to figure out if a variable being dereferenced has been assigned a
+null / non-null value on some or all paths through the program.
+</p>
+<p>
+Due to the inherent complexity, flow analysis is best performed in small chunks.
+Analyzing one method at a time can be done with good tool performance -
+whereas whole-system analysis is out of scope for the Eclipse Java compiler.
+The advantage is: analysis is fast and can be done incrementally
+such that the compiler can warn you directly as you type.
+The down-side: the analysis can not "see" which values (null or non-null) are
+flowing between methods (as parameters and return values).
+</p>
+<h2>Inter-prodedural null analysis</h2>
+<p>
+This is where null annotations come into play.
+By specifying a method parameter as <code>@NonNull</code> you can tell the compiler
+that you don't <em>want</em> a null value in this position.
+</p>
+<pre> String capitalize(@NonNull String in) {
+ return in.toUpperCase(); // no null check required
+ }
+ void caller(String s) {
+ if (s != null)
+ System.out.println(capitalize(s)); // preceding null check is required
+ }
+</pre>
+<p>
+In the vein of <strong>Design-by-Contract</strong> this has two sides:
+</p>
+<ol>
+<li>It's the <b>caller's</b> <b>responsibility</b> to never pass a null value,
+ which is to be ensured, e.g., by an explicit null check.</li>
+<li>The <b>implementor</b> of method <code>capitalize</code> enjoys the
+ <b>guarantee</b> that the argument <code>in</code> shall not be null
+ and thus dereferencing without a null check is OK here.
+</ol>
+<p>
+For method return values the situation is symmetric:
+</p>
+<pre> @NonNull String getString(String maybeString) {
+ return maybeString != null ? maybeString : "&lt;n/a&gt;"; // null check is required
+ }
+ void caller(String s) {
+ System.out.println(getString(s).toUpperCase()); // no null check required
+ }
+</pre>
+<ol>
+<li>Now the implementor must ensure that null is never returned.</li>
+<li>Conversely the caller now enjoys the guarantee that dereferencing the
+method result without checking is OK.
+</ol>
+<h2>Available annotations</h2>
+<p>
+The Eclipse Java compiler can be <a href="../reference/preferences/java/compiler/ref-preferences-errors-warnings.htm#null_analysis">configured</a> to use three distinct annotation types
+for its enhanced null analysis (which is disabled by default):
+</p>
+<ul>
+<li><code>@NonNull</code>: null is not a legal value</li>
+<li><code>@Nullable</code>: null value is allowed and must be expected</li>
+<li><code>@NonNullByDefault</code>: types in method signatures
+ that lack a null annotation are regarded as non-null.</li>
+</ul>
+<p>Annotations <code>@NonNull</code> and <code>@Nullable</code> are supported in these locations:</p>
+<ul>
+<li>Method parameter</li>
+<li>Method return (syntactically a method annotation is used here)</li>
+<li>Local variables</li>
+<li><em>(more to come in the future)</em></li>
+</ul>
+<p><code>@NonNullByDefault</code> is supported for</p>
+<ul>
+<li>Methods &ndash; to affect all types in this method's signature</li>
+<li>Types (classes, interfaces, enums) &ndash; to affect all methods in the type body</li>
+<li>Package (via a file <code>package-info.java</code>) &ndash; to affect all types in the package</li>
+</ul>
+<p>
+Note, that even the actual qualified names of these annotations are
+<a href="../reference/preferences/java/compiler/ref-preferences-errors-warnings.htm#null_annotation_names">configurable</a>,
+but by default the ones given above are used (from the package <code>org.eclipse.jdt.annotation</code>).
+</p>
+<h2>Interpretation of null annotations</h2>
+<p>
+It should be clear now that null annotations add more information to your
+Java program (which can then be used by the compiler to give better warnings).
+But what exactly do we want these annotations to say?
+From a pragmatic point of view there are at least three levels of what we might
+want to express with null annotations:
+<ol>
+<li>Sporadic hints to the reader (human and compiler)</li>
+<li>Design by contract: API specification for some or all methods</li>
+<li>Full specification using an extended type system</li>
+</ol>
+<p>
+For (1) you may start using null annotations right away and without reading further,
+but you shouldn't expect more than a little hint every now and then.
+The other levels deserve some more explaining.
+</p>
+<h2>Design by contract: API specification</h2>
+<p>
+At first sight using null annotations for API specifications in the vein of Design by Contract
+only means that the signatures of all API methods should be fully annotated,
+i.e., except for primitive types like <code>int</code> each parameter and each
+method return type should be marked as either <code>@NonNull</code> or <code>@Nullable</code>.
+Since this would mean to insert very many null annotations, it is good to know that in
+well-designed code (especially API methods), <code>@NonNull</code> is significantly more
+frequent than <code>@Nullable</code>. Thus the number of annotations can be reduced by
+declaring <code>@NonNull</code> as the <b>default</b>, using a <code>@NonNullByDefault</code>
+annotation at the package level.
+</p>
+<p>
+Note the significant difference between <code>@Nullable</code> and omitting a null annotation:
+This annotation explicitly states that null is OK and must be expected.
+By contrast, no annotation simply means, we don't know what's the intention.
+This is the old situation where sometimes both sides (caller and callee) redundantly check for null,
+and some times both sides wrongly assume that the other side will do the check.
+This is where NullPointerExceptions originate from.
+Without an annotation the compiler will not give specific advice, but with a
+<code>@Nullable</code> annotation every unchecked dereference will be flagged.
+</p>
+<p>
+With these basics we can directly map all parameter annotations to <b>pre-conditions</b> and
+interpret return annotations as <b>post-conditions</b> of the method.
+</p>
+<h3 id="override">Sub-typing and overriding</h3>
+<p>
+In object-oriented programming the concept of Design by Contract needs to address one more dimension:
+<b>sub-typing</b> and overriding (in the sequel the term "override" will be used in the sense of the
+<code>@Override</code> annotation in Java 6: methods overriding <em>or implementing</em> another method
+from a super-type). A client invoking a method like this:
+</p>
+<pre> @NonNull String checkedString(@Nullable String in)</pre>
+<p>
+should be allowed to assume that <em>all implementations</em> of this method fulfill the contract.
+So when the method declaration is found in an interface <code>I1</code>,
+we must rule out that any class <code>Cn</code> implementing <code>I1</code> provides
+an incompatible implementation. Specifically, it is illegal if any <code>Cn</code> tries
+to override this method with an implementation that declares the parameter as <code>@NonNull</code>.
+If we <em>would</em> allow this, a client module programmed against <code>I1</code> could legally
+pass null as the argument but the implementation would assume a non-null value &ndash;
+unchecked dereference inside the method implementation would be allowed but blow up at runtime.
+Hence, a <code>@Nullable</code> parameter specification <em>obliges all overrides</em>
+to admit null as an expected, legal value.
+</p>
+<p>
+Conversely, a <code>@NonNull</code> return specification <em>obliges all overrides</em>
+to ensure that null will never be returned.
+</p>
+<p>Therefore, the compiler has to check that no override adds a <code>@NonNull</code>
+parameter annotation (or a <code>@Nullable</code> return annotation) that didn't exist in the super-type.
+</p>
+<p>
+Interestingly, the reverse redefinitions are legal: adding a <code>@Nullable</code> parameter annotation
+or a <code>@NonNull</code> return annotation (you may consider these as "improvements" of the method,
+it accepts more values and produces a more specific return value).
+</p>
+<h3>Legacy super-types</h3>
+<p>
+The previous considerations add a difficulty when annotated code is written as a sub-type
+of a "legacy" (i.e., un-annotated) type (which may be from a 3rd party library, thus cannot be changed).
+If you read the last section very carefully you might have noticed that we cannot admit
+that a "legacy" method is overridden by a method with a <code>@NonNull</code> parameter
+(since clients using the super-type don't "see" the <code>@NonNull</code> obligation).
+</p>
+<p>
+In this situation you will be forced to omit null annotations
+(<em>plans exist to support adding annotations to libraries after-the-fact, but no promise can be made yet,
+if and when such a feature will be available</em>).
+</p>
+<h3>Canceling a nullness default</h3>
+<p>The situation will get tricky, if a sub-type of a "legacy" type resides in a package for which
+<code>@NonNullByDefault</code> has been specified. Now a type with an un-annotated super-type
+would need to mark all parameters in overriding methods as <code>@Nullable</code>:
+even omitting parameter annotations isn't allowed because that would be interpreted like a
+<code>@NonNull</code> parameter, which is prohibited in that position.
+That's why the Eclipse Java compiler supports cancellation
+of a nullness default: by annotating a method or type with <code>@NonNullByDefault(false)</code>
+an applicable default will be canceled for this element, and un-annotated parameters are again interpreted as
+unspecified. Now, sub-typing is legal again without adding unwanted <code>@Nullable</code>
+annotations.
+</p>
+<h3>Benefits of Design-by-Contract</h3>
+<p>
+Using null annotations in the style of Design-by-Contract as outlined above,
+helps to improve the <b>quality</b> of your Java code in several ways:
+At the interface between methods it is made explicit, which parameters / returns
+tolerate a null value and which ones don't. This captures design decisions,
+which are highly relevant to the developers, in a way that is also checkable by the compiler.
+</p>
+<p>
+Additionally, based on this interface specification the intra-procedural flow analysis can
+pick up available information and give much more precise errors/warnings.
+Without annotations any value flowing into or out of a method has unknown nullness
+and therefore null analysis remains silent about their usage.
+With API-level null annotations the nullness of most values is actually known,
+and significantly fewer NPEs will go unnoticed by the compiler.
+However, you should be aware that still some loopholes exist, where unspecified
+values flow into the analysis, preventing a complete statement whether NPEs can
+occur at runtime.
+</p>
+<h2>Complete specification using an extended type system</h2>
+<p>
+The support for null annotations has been designed in a way that is compatible to
+a future extension, whereby the compiler will be able to guarantee that a certified
+program will never throw a NullPointerException at runtime (with the typical caveats
+about uncheckable parts like reflection).
+While no promise can be made yet regarding such future extension, the basic idea
+is just a different interpretation of what we already have.
+</p>
+<p>
+By interpreting null annotations as part of the type system we double the number
+of types in a given program: for any given class <code>Cn</code>, the class itself
+is no longer considered as a legal type, but only "<code>@NonNull Cn</code>" and
+"<code>@Nullable Cn</code>" are useful types. So ideally for every value in a program
+we will know if it can be null (and must be checked before dereference) or not.
+If we exclude the third option (un-annotated types) then the compiler can rigorously
+flag <em>every</em> unsafe usage.
+</p>
+<p>
+The current implementation has two major limitations with regard to this goal:
+<ul>
+<li>Null annotations are not yet supported for fields.</li>
+<li>The Java syntax doesn't allow annotations in some relevant locations,
+ a limitation that is addressed by JSR-338 (scheduled for Java 8).</li>
+</ul>
+<p>
+As of today and despite the current limitations, the vision of fully specified and
+completely checked programs has already some implications that shaped the current
+analysis and the errors/warnings reported by the compiler.
+The details will be presented by explaining the rules that the compiler checks
+and the messages it issues upon violation of a rule.
+</p>
+<h2>Compiler messages explained</h2>
+<p>
+On the corresponding <a href="../reference/preferences/java/compiler/ref-preferences-errors-warnings.htm">preference page</a>
+the individual rules checked by the compiler are ground under the following headings:
+</p>
+<h3>Violation of null specification</h3>
+<p>As specification violation we handle any situation where a null annotation
+makes a claim that is violated by the actual implementation. The typical situation results
+from specifying a value (local, argument, method return) as <code>@NonNull</code> whereas
+the implementation actually provides a nullable value. Here an expression is considered as
+nullable if either it is statically known to evaluate to the value null, or if it is declared
+with a <code>@Nullable</code> annotation.
+</p>
+<p>Secondly, this group also contains the rules for method overriding as discussed
+<a href="#override">above</a>.
+Here a super method establishes a claim (e.g., that null is a legal argument)
+while an override tries to evade this claim (by assuming that null is <i>not</i> a legal argument).
+As mentioned even specializing an argument from un-annotated to <code>@NonNull</code>
+is a specification violation, because it introduces a contract that should bind the client
+(to not pass null), but a client using the super-type won't even see this contract,
+so he doesn't even know what is expected of him.
+</p>
+<p>
+The full list of situations regarded as specification violations is given
+<a href="../reference/preferences/java/compiler/ref-preferences-errors-warnings.htm#null_spec_violation">here</a>.
+It is important to understand that errors in this group <b>should never be ignored</b>,
+because otherwise the entire null analysis will be performed based on false assumptions.
+Specifically, whenever the compiler sees a value with a <code>@NonNull</code> annotation
+it takes it for granted that null will not occur at runtime.
+It's the rules about specification violations which ensure that this reasoning is sound.
+Therefore it is strongly recommended to leave this kind of problem configured as <b>errors</b>.
+</p>
+<h3>Conflict between null annotations and null inference</h3>
+<p>
+Also this group of rules watches over the adherence to null specifications.
+However, here we deal with values that are not <em>declared</em> as <code>@Nullable</code>
+(nor the value null itself), but values where the <b>intra-procedural flow analysis</b>
+<i>infers</i> that null can possibly occur on some execution path.
+</p>
+<p>
+This situation arises from the fact that for un-annotated local variables
+the compiler will infer whether null is possible using its flow analysis.
+Assuming that this analysis is accurate, if it sees a problem this problem has the
+same severity as direct violations of a null specification.
+Therefore, it is again strongly recommended to leave this problems configured as <b>errors</b>
+and not to ignore these messages.
+</p>
+<p>
+Creating a separate group for these problems serves two purposes: to document that
+a given problem was raised with the help of the flow analysis, and: to account for
+the fact that this flow analysis <em>could</em> be at fault (because of a bug in the
+implementation).
+For the case of an acknowledged implementation bug it could in exceptional situations
+be OK to suppress an error message of this kind.
+</p>
+<p>
+Given the nature of any static analysis, the flow analysis may fail to see that a
+certain combination of execution paths and values is not possible. As an example
+consider <em>variable correlation</em>:
+</p>
+<pre> String flatten(String[] inputs1, String[] inputs2) {
+ StringBuffer sb1 = null, sb2 = null;
+ int len = Math.min(inputs1.length, inputs2.length);
+ for (int i=0; i&lt;len; i++) {
+ if (sb1 == null) {
+ sb1 = new StringBuffer();
+ sb2 = new StringBuffer();
+ }
+ sb1.append(inputs1[i]);
+ sb2.append(inputs2[i]); // warning here
+ }
+ if (sb1 != null) return sb1.append(sb2).toString();
+ return "";
+ }
+</pre>
+<p>
+The compiler will report a potential null pointer access at the invocation of <code>sb2.append(..)</code>.
+The human reader can see that there is no actual danger because <code>sb1</code> and <code>sb2</code>
+actually correlate in a way that either both variables are null or both are not-null.
+At the line in question we know that <code>sb1</code> is not null, hence also <code>sb2</code>
+is not null. Without going into the details why such correlation analysis is beyond the capability
+of the Eclipse Java compiler, please just keep in mind that this analysis doesn't have the power of a
+full theorem prover and therefore pessimistically reports some problems which a more capable
+analysis could possibly identify as false alarms.
+</p>
+<p>
+If you want to benefit from flow analysis, you are advised to give a little help to the compiler
+so it can "see" your conclusions. This help can be as simple as splitting the <code>if (sb1 == null)</code>
+into two separate ifs, one for each local variable, which is a very small price to pay for the
+gain that now the compiler can exactly see what happens and check your code accordingly.
+More discussion on this topic will follow <a href="#tips_analyzable">below</a>.
+</p>
+<h3>Unchecked conversion from non-annotated type to @NonNull type</h3>
+<p>
+This group of problems is based on the following analogy: in a program using Java 5 generics
+any calls to pre-Java-5 libraries may expose raw types, i.e., applications of a generic type
+which fail to specify concrete type arguments. To fit such values into a program using
+generics the compiler can add an <i>implicit conversion</i> by assuming that type arguments were
+specified in the way expected by the client part of the code.
+The compiler will issue a warning about the use of such a conversion and proceed its type checking
+assuming the library "does the right thing".
+In exactly the same way, an un-annotated return type of a library method can be considered
+as a "raw" or "legacy" type.
+Again an <i>implicit conversion</i> can optimistically assume the expected specification.
+Again a warning is issued and analysis continues assuming that the library "does the right thing".
+</p>
+<p>
+Theoretically speaking, also the need for such implicit conversions indicates a specification
+violation. However, in this case it is 3rd party code that violates the specification which our
+code expects. Or, maybe (as we have convinced ourselves of) the 3rd party code does fulfill the
+contract, but only fails to declare so (because it doesn't use null annotations).
+In such situations we may not be able to exactly fix the problem for <em>organizational</em> reasons.
+</p>
+<pre> @SuppressWarnings("null")
+ @NonNull Foo foo = Library.getFoo(); // implicit conversion
+ foo.bar();
+</pre>
+<p>
+The above code snippet assumes that <code>Library.getFoo()</code> returns a <code>Foo</code>
+without specifying a null annotation. We can integrate the return value into our annotated
+program by assignment to a <code>@NonNull</code> local variable, which triggers a warning
+regarding an unchecked conversion.
+By adding a corresponding <code>SuppressWarnings("null")</code> to this declaration
+we acknowledge the inherent danger and accept the responsibility of having verified that
+the library actually behaves as desired.
+</p>
+<h2 id="tips_analyzable">Tips for making code better analyzable</h2>
+<p>
+If flow analysis cannot see that a value is indeed not null, the simplest strategy is always
+to add a new scoped local variable annotated with <code>@NonNull</code>.
+...
+</p>
+<h2 id="tips_adoption">Tips for adopting null annotations</h2>
+</body>
+</html>
diff --git a/bundles/org.eclipse.jdt.doc.user/topics_Tasks.xml b/bundles/org.eclipse.jdt.doc.user/topics_Tasks.xml
index 1e20489..a218455 100644
--- a/bundles/org.eclipse.jdt.doc.user/topics_Tasks.xml
+++ b/bundles/org.eclipse.jdt.doc.user/topics_Tasks.xml
@@ -114,6 +114,9 @@
<topic href="tasks/task-ant_javac_adapter.htm" label="Using the ant javac adapter"></topic>
<topic href="tasks/task-suppress_warnings.htm" label="Excluding warnings"></topic>
</topic>
+ <topic label="Improving Java code quality" href="tasks/task-improve_code_quality.htm">
+ <topic href="tasks/task-using_null_annotations.htm" label="Using null annotations"></topic>
+ </topic>
<topic label="Using the Formatter Application" href="tasks/tasks-230.htm">
<topic label="Running the formatter application" href="tasks/tasks-231.htm"></topic>
<topic label="Generating a config file for the formatter application" href="tasks/tasks-232.htm"></topic>