Skip to content

Conversation

@mernst
Copy link
Member

@mernst mernst commented Jan 25, 2020

Merge together with typetools/jdk#34 and plume-lib/plume-util#50

mernst added 30 commits January 17, 2020 16:19
…eters-may-be-null-not-enabled into collection-object-parameters-may-be-null
…:mernst/checker-framework into collection-object-parameters-may-be-null
Copy link
Contributor

@cpovirk cpovirk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't yet looked at the latest changes to the astub file.

Copy link
Member

@wmdietl wmdietl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these improvements!
A few comments from a quick look through the changes. I didn't think through the specification details, though.

changelog.txt Outdated
-AuseDefaultsForUncheckedCode to -AuseConservativeDefaultsForUncheckedCode
The old name works temporarily but will be removed in a future release.

For collection methods with `Object` formal parameter type, such as contains,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add Nullness Checker: or something, to make it clear which checkers are affected by this.

*/
@Pure
public int indexOf(@Nullable Object o) {
public int indexOf(@NonNull Object o) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit odd that the @NonNull is made explicit here (and before in the wildcard bounds).
NonNull is the default here and not used for non-Object parameter types.
Do you want these explicit? If so, maybe add a note in the README about this convention?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that you didn't make NonNull explicit in AbstractMap below...

import java.util.Map;
import org.checkerframework.checker.initialization.qual.*;
import org.checkerframework.checker.nullness.qual.*;
import org.checkerframework.checker.regex.qual.*;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why import all these annotations, even though they aren't used?
Why not import @Pure, which is used?

String line;
while ((line = reader.readLine()) != null) {
System.out.println(line);
try (InputStream in = getClass().getResourceAsStream("/git.properties")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not clear how this is related to this PR?

@mernst mernst merged commit 869caaa into typetools:master Mar 27, 2020
@mernst mernst deleted the collection-object-parameters-may-be-null branch March 27, 2020 20:59
cpovirk added a commit to cpovirk/typetoolsjdk that referenced this pull request Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants