Community
Participate
Working Groups
We'd love to have the following feature: non-null postconditions on given parameters. I'm not sure if that is feasable. But we have multiple patterns where that would be helpful. If computability complexity is a concern, demarking the interesting cases with an explicit assert statement is feasible and quite idiomatic. public static boolean isNullOrEmpty(String s) { if ( (s == null) || "".equals(s) ) { return true; } else { assert s != null; // should be picked up by outer analysis return false; } } This method is much very useful in cases like this: if (StringUtils.isNullOrEmpty(x)|| StringUtils.isNullOrEmpty(y)) { xAndYGood = false; } else { // current analysis does not know that x and y or NonNull if (x.equalsIgnoreCase(y)) { ... } } Also ATM we duplicate methods for cases where the result will be NonNull for a NonNull parameter; e.g. like this: public static @Nullable <T> T ifNull(@Nullable T returnIfNotNull, @Nullable T insteadOfNull) { if (returnIfNotNull == null) { return insteadOfNull; } assert returnIfNotNull != null; // should be picked up by outer analysis return returnIfNotNull; } public static @NonNull <T> T ifNullNN(@Nullable T returnIfNotNull, @NonNull T insteadOfNull) { if (returnIfNotNull == null) { return insteadOfNull; } return returnIfNotNull; } This use case is quite common with us. So common that in many case we don't do the duplication and loose the nullity-information. This duplication is ugly. In case some similar analysis would be added like e.g. for Emptiness, the count of necessary methods would rise exponentially.
(In reply to comment #0) > assert s != null; // should be picked up by outer analysis The one thing we certainly won't include is: whole-system analysis. That's why we introduced null annotations: any contract that goes beyond the limits of a single method must be specified explicitly. That's in fact the only way we can integrate our analysis with incremental compilation. Method ifNullNN() looks very useful to me as it is. OTOH, I don't understand what method ifNull is good for. You seem to be looking for a utility method that combines: (1) a null check (2) a custom check (3) support branching at the client side for handling the bad case Using methods similar to your ifNullNN() items (1) and (2) can be easily realized. What's missing is a second return value to flag the bad case, right? So, how about this utility method static @NonNull <T> T assertNotNullAndNotEmpty(@Nullable T o) { if (o != null && !"".equals(o)) return o; throw new NullPointerException("Assertion violated"); } and this client code: void client(@Nullable String s) { try { s = assertNotNullAndNotEmpty(s); System.out.println(s.toUpperCase()); } catch (NullPointerException npe) { // handle bad value for s } } This can be understood by the current analysis, requires no more than one helper method per custom check (here: not-empty). Let me know if I misunderstood your intention.
(In reply to comment #1) > (In reply to comment #0) > > assert s != null; // should be picked up by outer analysis > > The one thing we certainly won't include is: whole-system analysis. I totally agree. Im interested in relationships like e.g. in https://bugs.eclipse.org/bugs/show_bug.cgi?id=129907 (null-dependence on a final boolean), but in the context of method calls. What i suggested there is to add to the class/meta-information on compilation. And instead of manually adding some annotation like @Contract("returnIfNotNull != null -> result != null"); to let eclipse add such information based on assert statements before return. > > Method ifNullNN() looks very useful to me as it is. Yes it is. But the pattern of adding NN methods fails in the general case. > OTOH, I don't understand what method ifNull is good for. Cases where the default value is or could be null. // e.g. instead of if(newValue != null){ value = newValue; } value = ifNull(newValue,value); Main problem is, nullity depends on the context on the client side. As an example our code has a lot ignore-null semantics; e.g. transformation of optional parameters from webservice layer. In that case null-in means null-out. On sites where i know the in-parameter is non-null, i want to keep that information for later (as it helps keeping the client code clean). But the code is also used on sites where the the in-parameter could be null, so with the NN-pattern, i have to duplicate the methods to allow both cases. So, using the NN-pattern in the general case, I would have to at least double the number of methods. E.g. instead of 2 clone methods, I need 4. public static @Nullable CrmAddress cloneToCrmAddress(@Nullable Backend1Address address) { public static @Nullable CrmAddress cloneToCrmAddress(@Nullable Backend2Address address) { public static @NonNull CrmAddress cloneToCrmAddressNN(@NonNull Backend1Address address) { public static @NonNull CrmAddress cloneToCrmAddressNN(@NonNull Backend2Address address) { > > You seem to be looking for a utility method that combines: > (1) a null check > (2) a custom check > (3) support branching at the client side for handling the bad case > > Using methods similar to your ifNullNN() items (1) and (2) can be easily > realized. What's missing is a second return value to flag the bad case, > right? (3) is quite close to what I want. The problem is the 'bad case' is not a bad case, but usually the other good case and depending on context. Using exceptions for branching would work, but is very non-idiomatic (and kind of hard to read and bit of bloat). Don't think I could get this through a code review. And I'm not sure that I'd want to. And as I said, this is more of a general problem. Reusing dependencies declared between parameters which are enforced by a method call. Maybe this can be better understood as a of kind parameterized typing for nullity. E.g. // not possible in Java static <@Nullity @N> @N CrmAdress cloneToCrmAddress(@N Address address){ // IDE check for both cases: @N == @NonNull or @N == @Nullable if (adress == null) return null; else return new CrmAddress(address); } // use site void print(@NonNull Address address){ @NonNull CrmAddress address2 = cloneToCrmAddress(address); address2.doSomething(); } P.S. > > So, how about this utility method > > static @NonNull <T> T assertNotNullAndNotEmpty(@Nullable T o) { already have something like this, only composable: > static @NonNull <T> T assertNotNull(@Nullable T o) { > static @NonNull <T> T assertNotEmptyNN(@NonNull T o) { > and this client code: > @NonNull String s = assertNotEmptyNN(assertNotNull(maybeNullS));
(In reply to comment #2) > (In reply to comment #1) > > (In reply to comment #0) > > > assert s != null; // should be picked up by outer analysis > > > > The one thing we certainly won't include is: whole-system analysis. > I totally agree. > Im interested in relationships like e.g. in > https://bugs.eclipse.org/bugs/show_bug.cgi?id=129907 (null-dependence on a > final boolean), but in the context of method calls. > > What i suggested there where? > ... is to add to the class/meta-information on compilation. the compiler must not store in class files any synthetic meta-data that is not mandated by the spec. I tried hard arguing for a weaker variant (see bug 366063) but couldn't convince the project leads. I'm afraid the snippet I proposed in comment 1 may be the best we can offer for now (and it seems to fulfil all of your requirements - except for matters of style - but then you still have the option with pairs of methods). Once JSR 308 is integrated with null annotations we should check if annotating type arguments would allow some of the generalization you're seeking. As you mentioned bug 129907, please note that we already have a considerable number of RFEs marked [null][pattern] or [null][correlation] which we can't plan to address due to lack of man-power. Your request doesn't seem easier / more straight-forward. Sorry.
> the compiler must not store in class files any synthetic meta-data that is > not mandated by the spec. > > I tried hard arguing for a weaker variant (see bug 366063) but couldn't > convince the project leads. Ah, ok. Too bad. Maybe when external metadata profiles (which were thought for annotation of libraries) are useable, something could be done? > I'm afraid the snippet I proposed in comment 1 may be the best we can offer > for now I thought of a KISS variant - add lets say 'assert-equivalent-method-calls'. Actually for enabling correct workflow (if statements) with null-analysis, just a single method/pattern would be needed like /** @result true if arg is not null. As side effect, */ boolean isNotNull(T arg). // -> assert side-effect @Nullable String s = getS(); if (isNotNull(s) && isNotEmpty(/*NonNull*/s)){ /* NonNull s */ } else { /* Nullable s */ } I'm not sure if the analysis in the current implementation could figure out that s would be NonNull in the isNotEmpty call (as you are usually not able to add an assert statement inside the if-condition), but worst case a client could write if (isNotNull(s)) if(isNotEmpty(/*NonNull*/ s)) { /* NonNull s */ } The more sophisticated version of this could expose configuration for such 'assert-equivalent-method-calls' for arbitrary methods. One could add all those methods manually to configuration, which are then interpreted for null-analysis like a assert statement. Actually the easiest configuration would be: specify a method, type in an assert statement. IDE uses the assert statement in local analysis, so nothing would have to be added to the class files. This again would allow for imperative programming style. Do you think this would be possible and if I were to invest my casual fridays in eclipse null-analysis where would I start?
