Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Annotate some UI classes for nullness #86

Merged
merged 21 commits into from
Sep 29, 2020

Conversation

jpschewe
Copy link

@jpschewe jpschewe commented Sep 26, 2020

List of classes annotated for nullness:

  • JRootPane
  • JDialog
  • DefaultListCellRenderer
  • DefaultTreeCellRenderer
  • DefaultTableCellRenderer
  • UIDefaults
  • Locale
  • JFileChooser
  • JOptionPane
  • DefaultLookup
  • UIManager
  • JFrame
  • JComponent
  • awt.Frame
  • awt.Dialog
  • awt.Window
  • awt.Container
  • awt.Component

This includes:
* DefaultListCellRenderer
* DefaultTableCellRenderer
* DefaultTreeCellRender
* the classes that they call to get possibly null values
@jpschewe jpschewe changed the title Annotate javax.swing.Deafult*CellRenderer and javax.swing.JFileChooser for nullness Annotate Default*CellRenderer, JFileChooser, UIDefaults, UIManager, DefaultLookup for nullness Sep 26, 2020
@jpschewe
Copy link
Author

jpschewe commented Sep 26, 2020

@mernst
I see that the jtreg11 test for issue244/MyDialog.java is failing. I looked at issue typetools/checker-framework#244 and see that this error was fixed long ago and appears to be a problem with the checker framework. This pull request only modifies the annotated JDK. In this pull request I have added nullness annotations to JDialog and Window. Given the error message below, I'm wondering if they are related.

- compiler.err.proc.messager: Exception while parsing annotated-jdk/src/java.desktop/share/classes/java/awt/Window.java: AnnotatedTypeMirror.createType: input should type-check already. Found error type: sun.awt.util.IdentityArrayList<java.awt.Window>
; The Checker Framework crashed.  Please report the crash.
Compilation unit: /home/jpschewe/projects/checker-framework/checker-framework/checker/jtregJdk11/issue244/MyDialog.java
Last visited tree at line 7 column 1:
public class MyDialog extends javax.swing.JDialog {}
Exception: org.checkerframework.javacutil.BugInCF: AnnotatedTypeMirror.createType: input should type-check already. Found error type: sun.awt.util.IdentityArrayList<java.awt.Window>; org.checkerframework.javacutil.BugInCF: AnnotatedTypeMirror.createType: input should type-check already. Found error type: sun.awt.util.IdentityArrayList<java.awt.Window>
	at org.checkerframework.framework.type.AnnotatedTypeMirror.createType(AnnotatedTypeMirror.java:78)
	at org.checkerframework.framework.type.AnnotatedTypeMirror$AnnotatedExecutableType.getReturnType(AnnotatedTypeMirror.java:1156)
	at org.checkerframework.framework.util.element.MethodApplier.extractAndApply(MethodApplier.java:125)
	at org.checkerframework.framework.util.element.MethodApplier.apply(MethodApplier.java:32)
	at org.checkerframework.framework.type.ElementAnnotationApplier.applyInternal(ElementAnnotationApplier.java:129)
	at org.checkerframework.framework.type.ElementAnnotationApplier.apply(ElementAnnotationApplier.java:78)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.fromElement(AnnotatedTypeFactory.java:1158)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.fromElement(AnnotatedTypeFactory.java:2403)
	at org.checkerframework.framework.stub.StubParser.processCallableDeclaration(StubParser.java:810)
	at org.checkerframework.framework.stub.StubParser.processTypeDecl(StubParser.java:644)
	at org.checkerframework.framework.stub.StubParser.processCompilationUnit(StubParser.java:570)
	at org.checkerframework.framework.stub.StubParser.processStubUnit(StubParser.java:553)
	at org.checkerframework.framework.stub.StubParser.process(StubParser.java:543)
	at org.checkerframework.framework.stub.StubParser.parse(StubParser.java:501)
	at org.checkerframework.framework.stub.StubParser.parseJdkFileAsStub(StubParser.java:472)
	at org.checkerframework.framework.stub.StubTypes.parseJarEntry(StubTypes.java:416)
	at org.checkerframework.framework.stub.StubTypes.parseEnclosingClass(StubTypes.java:342)
	at org.checkerframework.framework.stub.StubTypes.getDeclAnnotation(StubTypes.java:316)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotations(AnnotatedTypeFactory.java:3130)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.inheritOverriddenDeclAnnosFromTypeDecl(AnnotatedTypeFactory.java:3162)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotations(AnnotatedTypeFactory.java:3137)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.inheritOverriddenDeclAnnosFromTypeDecl(AnnotatedTypeFactory.java:3162)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotations(AnnotatedTypeFactory.java:3137)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.inheritOverriddenDeclAnnosFromTypeDecl(AnnotatedTypeFactory.java:3162)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotations(AnnotatedTypeFactory.java:3137)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotation(AnnotatedTypeFactory.java:3058)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getDeclAnnotation(AnnotatedTypeFactory.java:2980)
	at org.checkerframework.framework.type.typeannotator.DefaultQualifierForUseTypeAnnotator.getSupportAnnosFromDefaultQualifierForUses(DefaultQualifierForUseTypeAnnotator.java:100)
	at org.checkerframework.framework.type.typeannotator.DefaultQualifierForUseTypeAnnotator.getDefaultAnnosForUses(DefaultQualifierForUseTypeAnnotator.java:55)
	at org.checkerframework.framework.type.typeannotator.DefaultQualifierForUseTypeAnnotator.visitDeclared(DefaultQualifierForUseTypeAnnotator.java:32)
	at org.checkerframework.framework.type.typeannotator.DefaultQualifierForUseTypeAnnotator.visitDeclared(DefaultQualifierForUseTypeAnnotator.java:22)
	at org.checkerframework.framework.type.AnnotatedTypeMirror$AnnotatedDeclaredType.accept(AnnotatedTypeMirror.java:902)
	at org.checkerframework.framework.type.visitor.AnnotatedTypeScanner.scan(AnnotatedTypeScanner.java:199)
	at org.checkerframework.framework.type.visitor.AnnotatedTypeScanner.visit(AnnotatedTypeScanner.java:187)
	at org.checkerframework.framework.type.visitor.AnnotatedTypeScanner.visit(AnnotatedTypeScanner.java:174)
	at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.addAnnotationsFromDefaultForType(GenericAnnotatedTypeFactory.java:2010)
	at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.addComputedTypeAnnotations(GenericAnnotatedTypeFactory.java:1793)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getAnnotatedType(AnnotatedTypeFactory.java:969)
	at org.checkerframework.framework.type.AnnotatedTypeFactory.getAnnotatedType(AnnotatedTypeFactory.java:2376)
	at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.performFlowAnalysis(GenericAnnotatedTypeFactory.java:1112)
	at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.checkAndPerformFlowAnalysis(GenericAnnotatedTypeFactory.java:1615)
	at org.checkerframework.framework.type.GenericAnnotatedTypeFactory.preProcessClassTree(GenericAnnotatedTypeFactory.java:293)
	at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:318)
	at org.checkerframework.common.basetype.BaseTypeVisitor.visitClass(BaseTypeVisitor.java:171)
	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCClassDecl.accept(JCTree.java:808)
	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(TreePathScanner.java:56)
	at org.checkerframework.framework.source.SourceVisitor.visit(SourceVisitor.java:82)
	at org.checkerframework.framework.source.SourceChecker.typeProcess(SourceChecker.java:916)
	at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:513)
	at org.checkerframework.common.basetype.BaseTypeChecker.typeProcess(BaseTypeChecker.java:506)
	at org.checkerframework.javacutil.AbstractTypeProcessor$AttributionTaskListener.finished(AbstractTypeProcessor.java:190)
	at jdk.compiler/com.sun.tools.javac.api.ClientCodeWrapper$WrappedTaskListener.finished(ClientCodeWrapper.java:828)
	at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(MultiTaskListener.java:132)
	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1418)
	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(JavaCompiler.java:1375)
	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:973)
	at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:311)
	at jdk.compiler/com.sun.tools.javac.main.Main.compile(Main.java:170)
	at jdk.compiler/com.sun.tools.javac.Main.compile(Main.java:75)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at com.sun.javatest.regtest.agent.RegressionCompileCommand.run(RegressionCompileCommand.java:190)
	at com.sun.javatest.regtest.agent.CompileActionHelper.runCompile(CompileActionHelper.java:91)
	at com.sun.javatest.regtest.agent.AgentServer.doCompile(AgentServer.java:210)
	at com.sun.javatest.regtest.agent.AgentServer.run(AgentServer.java:180)
	at com.sun.javatest.regtest.agent.AgentServer.main(AgentServer.java:59)

The way that the cache is setup in Locale, get cannot return null since
createObject cannot return null and null keys are not allowed.
It's an internal sun class and shouldn't be used outside. I had started
annotating it to help with the annotations for Locale and realized that
this was a mistake.
@jpschewe jpschewe changed the title Annotate Default*CellRenderer, JFileChooser, UIDefaults, UIManager, DefaultLookup for nullness Annotate some UI classes for nullness Sep 27, 2020
@mernst
Copy link
Member

mernst commented Sep 28, 2020

The build is failing only because of the changes in Window.java. Could you move those changes into a different branch and undo them in this pull request? Then we can merge this pull request and independently investigate why the changes in Window.java cause tests to fail under JDK 11 but not under JDK 8.

These changes have been moved to PR typetools#89.
@@ -532,7 +533,7 @@ public void setSelectedFile(File file) {
*/
@BeanProperty(description
= "The list of selected files if the chooser is in multiple selection mode.")
public void setSelectedFiles(File[] selectedFiles) {
public void setSelectedFiles(@Nullable File[] selectedFiles) {
Copy link
Member

Choose a reason for hiding this comment

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

Here, too, @Nullable should be on the array type.

Copy link
Author

Choose a reason for hiding this comment

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

Here the array itself can be null, so I've pushed a fix for this.
The elements of the array "may" be null, but probably shouldn't be null.

private File[] selectedFiles;
private @Nullable File currentDirectory = null;
private @Nullable File selectedFile = null;
private @Nullable File[] selectedFiles;
Copy link
Member

Choose a reason for hiding this comment

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

Based on the code, I believe this should be File @Nullable [] (a possibly-null array of non-null files) rather than @Nullable File[] (a non-null array of possibly-null files).

Copy link
Author

Choose a reason for hiding this comment

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

Agreed, pending the comment above about possibly allowing nulls inside the array.

Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

This looks good (thanks!), except for a concern about an array type.

@jpschewe jpschewe requested a review from mernst September 29, 2020 20:40
Copy link
Member

@mernst mernst left a comment

Choose a reason for hiding this comment

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

This looks good now. Thanks a lot for these annotations!

@mernst mernst merged commit ff3a008 into typetools:master Sep 29, 2020
@jpschewe jpschewe deleted the annotate-swing-part-1 branch September 29, 2020 22:52
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.

2 participants