From 3a3bb2ce292fcfb8ad34408d2ac7106d05ba642f Mon Sep 17 00:00:00 2001 From: Terry Parker Date: Mon, 9 Nov 2015 13:40:39 -0800 Subject: [PATCH 1/3] Add a JGit-internal Nullable type Commit 847b3d1 enabled annotation-based NPE analysis to JGit. In so doing, it introduced a new dependency on the org.eclipse.jdt that is undesirable. Follow Gerrit's lead by adding an internal Nullable type (see https://gerrit.googlesource.com/gerrit/+/stable-2.11/gerrit-common/src/main/java/com/google/gerrit/common/Nullable.java). The javax.annotations.Nullable class uses a retention policy of RUNTIME, whereas the org.eclipse.jdt.annotation.Nullable class used a policy of CLASS. Since I'm not aware of tools that make use of the annotation at runtime I chose the CLASS policy. If in the future there is a benefit to retaining the annotations in the running binary we can update this class. Change-Id: I63dc8f9a6dc46b517cbde211bb5e2f8521a54d04 Signed-off-by: Terry Parker --- .../eclipse/jgit/annotations/Nullable.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java b/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java new file mode 100644 index 000000000..254920e7a --- /dev/null +++ b/org.eclipse.jgit/src/org/eclipse/jgit/annotations/Nullable.java @@ -0,0 +1,67 @@ +/* + * Copyright (C) 2015, Google Inc. + * and other copyright owners as documented in the project's IP log. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.annotations; + +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.LOCAL_VARIABLE; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * JGit's replacement for the {@code javax.annotations.Nullable}. + *

+ * Denotes that a local variable, parameter, field, method return value can be + * {@code null}. + */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ FIELD, METHOD, PARAMETER, LOCAL_VARIABLE }) +public @interface Nullable { + // marker annotation with no members +} From 6ddb8e703bae2a74287bb5ac7ae94460324766cf Mon Sep 17 00:00:00 2001 From: Terry Parker Date: Mon, 9 Nov 2015 13:42:20 -0800 Subject: [PATCH 2/3] Use the JGit-internal @Nullable annotation Update existing @Nullable uses to use the JGit-internal version of the annotation. Change-Id: I95234ffad4c3110f9597fbd3a2760f653e22ecf7 Signed-off-by: Terry Parker --- org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java index bcaf62a0c..e407dd8be 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/util/FS.java @@ -65,7 +65,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import org.eclipse.jdt.annotation.Nullable; +import org.eclipse.jgit.annotations.Nullable; import org.eclipse.jgit.api.errors.JGitInternalException; import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.lib.Constants; @@ -404,7 +404,8 @@ public abstract class FS { * as component array * @param encoding * to be used to parse the command's output - * @return the one-line output of the command + * @return the one-line output of the command or {@code null} if there is + * none */ @Nullable protected static String readPipe(File dir, String[] command, String encoding) { @@ -423,7 +424,8 @@ public abstract class FS { * @param env * Map of environment variables to be merged with those of the * current process - * @return the one-line output of the command + * @return the one-line output of the command or {@code null} if there is + * none * @since 4.0 */ @Nullable From ca3ea18371d95a38738156287f300a289fb2c3ff Mon Sep 17 00:00:00 2001 From: Terry Parker Date: Mon, 9 Nov 2015 14:19:11 -0800 Subject: [PATCH 3/3] Update dependencies to use the JGit-internal @Nullable Update the project-specific Eclipse settings to replace the use of the org.eclipse.jdt.annotation.Nullable class the new JGit-specific @Nullable annotation. I verified that Eclipse reports errors when the return value of a method annotated with @org.eclipse.jgit.annotations.Nullable is dereferenced without a null check. Also remove the Maven and MANIFEST.MF dependencies on org.eclipse.jdt.annotation. Eclipse null analysis uses three annotations: @Nullable, @NonNull and @NonNullByDefault. All three are updated in this patch because it is invalid to set the Eclipse preferences to empty values. So far only @Nullable has been introduced in org.eclipse.jgit.annotations. My personal preference is to follow the advice in Effective Java and avoid the null-return idiom, and to avoid passing null values in general. This sets the expectation is that arguments and return types are assumed non-null unless otherwise documented. If that is the expectation, then consistent application of @NonNull is redundant and hurts readability by cluttering the code, obscuring the occasional @Nullable annotation that really requires attention. If the JGit community decides there is value in using the @NonNull and @NonNullByDefault annotations we can add them--this change configures Eclipse to use them. Change-Id: I9af1b786d1b44b9b0d9c609480dc842df79bf698 Signed-off-by: Terry Parker --- .../.settings/org.eclipse.jdt.core.prefs | 6 +++--- org.eclipse.jgit.archive/META-INF/MANIFEST.MF | 1 - org.eclipse.jgit.pgm/.settings/org.eclipse.jdt.core.prefs | 6 +++--- org.eclipse.jgit.pgm/META-INF/MANIFEST.MF | 1 - org.eclipse.jgit/.settings/org.eclipse.jdt.core.prefs | 6 +++--- org.eclipse.jgit/META-INF/MANIFEST.MF | 3 +-- org.eclipse.jgit/pom.xml | 6 ------ 7 files changed, 10 insertions(+), 19 deletions(-) diff --git a/org.eclipse.jgit.archive/.settings/org.eclipse.jdt.core.prefs b/org.eclipse.jgit.archive/.settings/org.eclipse.jdt.core.prefs index 4e28e0b26..45d6d2c4c 100644 --- a/org.eclipse.jgit.archive/.settings/org.eclipse.jdt.core.prefs +++ b/org.eclipse.jgit.archive/.settings/org.eclipse.jdt.core.prefs @@ -1,9 +1,9 @@ eclipse.preferences.version=1 org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore -org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonNull -org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jdt.annotation.NonNullByDefault -org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jdt.annotation.Nullable +org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jgit.annotations.NonNull +org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jgit.annotations.NonNullByDefault +org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jgit.annotations.Nullable org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate diff --git a/org.eclipse.jgit.archive/META-INF/MANIFEST.MF b/org.eclipse.jgit.archive/META-INF/MANIFEST.MF index e8aad55c0..2bd016422 100644 --- a/org.eclipse.jgit.archive/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.archive/META-INF/MANIFEST.MF @@ -24,4 +24,3 @@ Export-Package: org.eclipse.jgit.archive;version="4.2.0"; org.eclipse.jgit.api, org.apache.commons.compress.archivers, org.osgi.framework" -Require-Bundle: org.eclipse.jdt.annotation;bundle-version="[1.1.0,2.0.0)";resolution:=optional diff --git a/org.eclipse.jgit.pgm/.settings/org.eclipse.jdt.core.prefs b/org.eclipse.jgit.pgm/.settings/org.eclipse.jdt.core.prefs index 4e28e0b26..45d6d2c4c 100644 --- a/org.eclipse.jgit.pgm/.settings/org.eclipse.jdt.core.prefs +++ b/org.eclipse.jgit.pgm/.settings/org.eclipse.jdt.core.prefs @@ -1,9 +1,9 @@ eclipse.preferences.version=1 org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore -org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonNull -org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jdt.annotation.NonNullByDefault -org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jdt.annotation.Nullable +org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jgit.annotations.NonNull +org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jgit.annotations.NonNullByDefault +org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jgit.annotations.Nullable org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate diff --git a/org.eclipse.jgit.pgm/META-INF/MANIFEST.MF b/org.eclipse.jgit.pgm/META-INF/MANIFEST.MF index 1916dec53..567fd0575 100644 --- a/org.eclipse.jgit.pgm/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit.pgm/META-INF/MANIFEST.MF @@ -63,4 +63,3 @@ Export-Package: org.eclipse.jgit.console;version="4.2.0"; org.kohsuke.args4j" Main-Class: org.eclipse.jgit.pgm.Main Implementation-Title: JGit Command Line Interface -Require-Bundle: org.eclipse.jdt.annotation;bundle-version="[1.1.0,2.0.0)";resolution:=optional diff --git a/org.eclipse.jgit/.settings/org.eclipse.jdt.core.prefs b/org.eclipse.jgit/.settings/org.eclipse.jdt.core.prefs index 4e28e0b26..195987db6 100644 --- a/org.eclipse.jgit/.settings/org.eclipse.jdt.core.prefs +++ b/org.eclipse.jgit/.settings/org.eclipse.jdt.core.prefs @@ -1,9 +1,9 @@ eclipse.preferences.version=1 org.eclipse.jdt.core.compiler.annotation.inheritNullAnnotations=enabled org.eclipse.jdt.core.compiler.annotation.missingNonNullByDefaultAnnotation=ignore -org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jdt.annotation.NonNull -org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jdt.annotation.NonNullByDefault -org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jdt.annotation.Nullable +org.eclipse.jdt.core.compiler.annotation.nonnull=org.eclipse.jgit.annotations.NonNull +org.eclipse.jdt.core.compiler.annotation.nonnullbydefault=org.eclipse.jgit.annotations.NonByDefault +org.eclipse.jdt.core.compiler.annotation.nullable=org.eclipse.jgit.annotations.Nullable org.eclipse.jdt.core.compiler.annotation.nullanalysis=enabled org.eclipse.jdt.core.compiler.codegen.inlineJsrBytecode=enabled org.eclipse.jdt.core.compiler.codegen.methodParameters=do not generate diff --git a/org.eclipse.jgit/META-INF/MANIFEST.MF b/org.eclipse.jgit/META-INF/MANIFEST.MF index 4199cc6f9..b6899b280 100644 --- a/org.eclipse.jgit/META-INF/MANIFEST.MF +++ b/org.eclipse.jgit/META-INF/MANIFEST.MF @@ -154,8 +154,7 @@ Export-Package: org.eclipse.jgit.api;version="4.2.0"; org.ietf.jgss", org.eclipse.jgit.util.io;version="4.2.0" Bundle-RequiredExecutionEnvironment: JavaSE-1.7 -Require-Bundle: com.jcraft.jsch;bundle-version="[0.1.37,0.2.0)", - org.eclipse.jdt.annotation;bundle-version="[1.1.0,2.0.0)";resolution:=optional +Require-Bundle: com.jcraft.jsch;bundle-version="[0.1.37,0.2.0)" Import-Package: com.googlecode.javaewah;version="[0.7.9,0.8.0)", javax.crypto, javax.net.ssl, diff --git a/org.eclipse.jgit/pom.xml b/org.eclipse.jgit/pom.xml index 1eac7e499..4f98323e5 100644 --- a/org.eclipse.jgit/pom.xml +++ b/org.eclipse.jgit/pom.xml @@ -88,12 +88,6 @@ org.slf4j slf4j-api - - - org.eclipse.jdt - org.eclipse.jdt.annotation - 1.1.0 -