Crash on valid Java source file

[kelloggm]

Trying to reduce nullaway/nullaway #1238 via this command:

./gradlew run --args='--modularityModel nullaway --outputDirectory "/Users/mjk76/Research/specimin/playground/nullaway-1238-out" --root "/Users/mjk76/Research/specimin/playground/nullaway-1238/src/" --targetFile "org/chromium/chrome/browser/creator/CreatorMediator.java" --targetMethod "org.chromium.chrome.browser.creator.CreatorMediator#followClickHandler()"' -PskipCheckerFramework=true

results in a crash:

> Task :run FAILED
Exception in thread "main" java.lang.RuntimeException: Need to check this field access expression with isAnUnsolvedStaticFieldCalledByAQualifiedClassName before using this method
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.updateClassSetWithQualifiedFieldSignature(UnsolvedSymbolVisitor.java:3533)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.updateSyntheticClassWithNonStaticFields(UnsolvedSymbolVisitor.java:2364)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:1087)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.expr.FieldAccessExpr.accept(FieldAccessExpr.java:95)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:153)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.expr.BinaryExpr.accept(BinaryExpr.java:140)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:599)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.stmt.IfStmt.accept(IfStmt.java:83)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:166)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:775)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.stmt.BlockStmt.accept(BlockStmt.java:71)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1071)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:837)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.expr.LambdaExpr.accept(LambdaExpr.java:162)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:642)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:1213)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.expr.MethodCallExpr.accept(MethodCallExpr.java:135)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:475)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.stmt.ExpressionStmt.accept(ExpressionStmt.java:72)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:166)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:775)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.stmt.BlockStmt.accept(BlockStmt.java:71)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:607)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.stmt.IfStmt.accept(IfStmt.java:83)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:166)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:775)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.stmt.BlockStmt.accept(BlockStmt.java:71)
        at com.github.javaparser.ast.visitor.ModifierVisitor.lambda$visit$60(ModifierVisitor.java:661)
        at java.base/java.util.Optional.map(Optional.java:260)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:661)
        at org.checkerframework.specimin.SpeciminStateVisitor.visit(SpeciminStateVisitor.java:187)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.processMethodDeclaration(UnsolvedSymbolVisitor.java:2128)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:1046)
        at org.checkerframework.specimin.UnsolvedSymbolVisitor.visit(UnsolvedSymbolVisitor.java:106)
        at com.github.javaparser.ast.body.MethodDeclaration.accept(MethodDeclaration.java:195)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:239)
        at org.checkerframework.specimin.SpeciminStateVisitor.visit(SpeciminStateVisitor.java:216)
        at org.checkerframework.specimin.SpeciminStateVisitor.visit(SpeciminStateVisitor.java:33)
        at com.github.javaparser.ast.body.ClassOrInterfaceDeclaration.accept(ClassOrInterfaceDeclaration.java:152)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:1111)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.NodeList.accept(NodeList.java:275)
        at com.github.javaparser.ast.visitor.ModifierVisitor.modifyList(ModifierVisitor.java:1149)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:307)
        at com.github.javaparser.ast.visitor.ModifierVisitor.visit(ModifierVisitor.java:52)
        at com.github.javaparser.ast.CompilationUnit.accept(CompilationUnit.java:134)
        at org.checkerframework.specimin.SpeciminRunner.performMinimizationImpl(SpeciminRunner.java:305)
        at org.checkerframework.specimin.SpeciminRunner.performMinimization(SpeciminRunner.java:164)
        at org.checkerframework.specimin.SpeciminRunner.main(SpeciminRunner.java:91)

FAILURE: Build failed with an exception.

Input file is here: https://source.chromium.org/chromium/chromium/src/+/main:chrome/browser/creator/android/java/src/org/chromium/chrome/browser/creator/CreatorMediator.java?q=symbol:%5Cborg.chromium.chrome.browser.creator.CreatorMediator.followClickHandler%5Cb%20case:yes

[theron-wang]

@kelloggm With some small fixes, the refactor does successfully create an output, but it's not compilable. There are two reasons for this:

1. Final fields have no initializers

private final PropertyModel mCreatorModel;
    private final CreatorSnackbarController mCreatorSnackbarController;
    private final SignInInterstitialInitiator mSignInInterstitialInitiator;

    CreatorMediator(
            Context context,
            PropertyModel creatorModel,
            CreatorSnackbarController creatorSnackbarController,
            SignInInterstitialInitiator signInInterstitialInitiator) {
        mCreatorModel = creatorModel;
        mCreatorSnackbarController = creatorSnackbarController;
        mSignInInterstitialInitiator = signInInterstitialInitiator;
    }

Since the constructor isn't preserved, the final fields are never set, so the compiler complains. Would the best way around this be to set these fields to null if no constructor is preserved to set them?

2. Lambda types

(result) -> {
                        if (result.requestStatus == SUCCESS) {
                            mCreatorModel.set(CreatorProperties.IS_FOLLOWED_KEY, true);
                        }
                        mCreatorSnackbarController.showSnackbarForFollow(
                                result.requestStatus,
                                mCreatorModel.get(CreatorProperties.TITLE_KEY));
                    }

We can successfully generate a type for result, but our handling of lambdas right now just sets all parameters to ?. Perhaps the way is to always generate synthetic types for unknown lambda parameter types, and include those in the lambda instead of ??

[kelloggm]

Since the constructor isn't preserved, the final fields are never set, so the compiler complains. Would the best way around this be to set these fields to null if no constructor is preserved to set them?

Yes, that's probably the right thing to do. It would also be acceptable to preserve the constructor because it assigns preserved fields that are final.

[kelloggm]

our handling of lambdas right now just sets all parameters to ?

Does this prevent compilation? If so, what's the compilation error?

[theron-wang]

Does this prevent compilation? If so, what's the compilation error?

Yes, it does: since it's ?, result is Object, and requestStatus does not exist on Object.

The exact compilation error is this:

org\chromium\chrome\browser\creator\CreatorMediator.java:27: error: cannot find symbol
            if (result.requestStatus == SUCCESS) {
                      ^
  symbol:   variable requestStatus
  location: variable result of type Object
org\chromium\chrome\browser\creator\CreatorMediator.java:31: error: cannot find symbol
                result.requestStatus, mCreatorModel.get(CreatorProperties.TITLE_KEY));
                      ^
  symbol:   variable requestStatus
  location: variable result of type Object
2 errors

[kelloggm]

Yes, it does: since it's ?, result is Object, and requestStatus does not exist on Object.

That makes sense. It seems like we need to constrain lambda parameter types based on how they are used, similar to how we constrain type parameters or field types from superclasses, which I think is what you were suggesting:

Perhaps the way is to always generate synthetic types for unknown lambda parameter types, and include those in the lambda instead of ?