Giter Site home page Giter Site logo

nullawayannotator's Introduction

NullAwayAnnotator Build Status

NullAwayAnnotator, or simply Annotator, is a tool that automatically infers nullability types in the given source code and injects the corresponding annotations to pass NullAway checks.

Applying NullAway to build systems requires manual effort in annotating the source code. Even if the code is free of nullability errors, annotations are still needed to pass NullAway checks. A tool that can automatically infer types in the source code and inject the corresponding annotations to pass NullAway checks can significantly reduce the effort of integrating NullAway into build systems.

Annotator minimizes the number of reported NullAway errors by inferring nullability types of elements in the source code and injecting the corresponding annotations. For errors that are not resolvable with any annotations, Annotator injects appropriate suppression annotations. The final output of Annotator is a source code that passes NullAway checks with no remaining errors.

Code Example

In the code below, NullAway reports five warnings.

class Test{
    Object f1 = null; // warning: assigning @Nullable expression to @NonNull field
    Object f2 = null; // warning: assigning @Nullable expression to @NonNull field
    Object f3 = null; // warning: assigning @Nullable expression to @NonNull field
    Object f4 = null; // warning: assigning @Nullable expression to @NonNull field
    Object f5 = f4;
    Object f6 = new Object();
    
    String m1(){
        return f1 != null ? f1.toString() : f2.toString() + f6.toString();
    }
    
    int m2(){
        return f3 != null ? f3.hashCode() : f2.hashCode() + f6.hashCode();
    }
    
    Object m3(){
        return f5;
    }
    
    void m4(){
         f6 = null; // warning: assigning @Nullable expression to @NonNull field
    }
}

Annotator can infer the nullable types in the code above and inject the corresponding annotations. For unresolved errors, suppression annotations are injected. The output below shows the result of running Annotator on the code above.

import javax.annotation.Nullable; // added by Annotator
import org.jspecify.annotations.NullUnmarked; // added by Annotator

class Test{
    @Nullable Object f1 = null;
    @SuppressWarnings("NullAway") Object f2 = null; // inferred to be @Nonnull, and null assignment is suppressed.
    @Nullable Object f3 = null;
    @Nullable Object f4 = null;
    @Nullable Object f5 = f4;
    Object f6 = new Object();  // inferred to be @Nonnull
    
    String m1(){
        return f1 != null ? f1.toString() : f2.toString() + f6.toString();
    }
    
    int m2(){
        return f3 != null ? f3.hashCode() : f2.hashCode() + f6.hashCode();
    }
    
    @Nullable Object m3(){ // inferred to be @Nullable as a result of f5 being @Nullable.
        return f5;
    }
    
    @NullUnmarked //f6 is inferred to be @Nonnull, but it is assigned to null. The error is suppressed by @NullUnmarked.
    void m4(){
         f6 = null; 
    }
}

Annotator propagates the effects of a change throughout the entire module and injects several follow-up annotations to fully resolve a specific warning. It is also capable of processing modules within monorepos, taking into account the modules public APIs and the impacts of annotations on downstream dependencies for improved results.

Installation

We ship Annotator on Maven, as a JAR. You can find the artifact information below -

GROUP: edu.ucr.cs.riple.annotator
ID: annotator-core
ID: annotator-scanner

Using Annotator on a target Java Project

This sections describes how to run Annotator on any project.

  • Requirements for the Target Project

    Dependencies

    • NullAway checker must be activated with version >= 0.10.10
    • AnnotatorScanner checker must be activated with version >= 1.3.6, see more about AnnotatorScanner here.

    Error Prone Flags

    Since Nullaway is built as a plugin for Error Prone, we need to set the following flags in our build.gradle,

    "-Xep:NullAway:ERROR", // to activate NullAway
    "-XepOpt:NullAway:SerializeFixMetadata=true",
    "-XepOpt:NullAway:FixSerializationConfigPath=path_to_nullaway_config.xml",
    "-Xep:AnnotatorScanner:ERROR", // to activate Annotator AnnotatorScanner
    "-XepOpt:AnnotatorScanner:ConfigPath=path_to_scanner_config.xml",
    

    The following code snippet demonstrates how to configure the JavaCompile tasks in your build.gradle to use NullAway as a plugin for Error Prone:

     dependencies {  
     	annotationProcessor 'edu.ucr.cs.riple.annotator:annotator-scanner:1.3.6'  
     	annotationProcessor "com.uber.nullaway:nullaway:0.10.10"  
     	errorprone "com.google.errorprone:error_prone_core:2.4.0"  
     	errorproneJavac "com.google.errorprone:javac:9+181-r4173-1"  
     	//All other target project dependencies
     }  
       
       
     tasks.withType(JavaCompile) {  
     	// remove the if condition if you want to run NullAway on test code  
     	if (!name.toLowerCase().contains("test")) {  
     		options.errorprone {  
     		check("NullAway", CheckSeverity.ERROR)  
     		check("AnnotatorScanner", CheckSeverity.ERROR)  
     		option("NullAway:AnnotatedPackages", "org.example")  
     		option("NullAway:SerializeFixMetadata", "true")  
     		option("NullAway:FixSerializationConfigPath", "path_to/nullaway.xml")  
     		option("AnnotatorScanner:ConfigPath", "path_to/scanner.xml")  
     		}  
     		options.compilerArgs << "-Xmaxerrs"<< "100000"  
     		options.compilerArgs << "-Xmaxwarns" << "100000"  
     	}  
       
     }

    path_to_nullaway_config.xml and path_to_scanner_config.xml are configuration files that do not need to be created during the initial project setup. The script will generate these files, facilitating seamless communication between the script and the analysis. At this point, the target project is prepared for the Annotator to process.

    You must provide the Annotator with the paths to path_to_nullaway_config.xml and path_to_scanner_config.xml. Further details on this process are described in the sections below.

  • Running Annotator

    Annotator necessitates specific flag values for successful execution. You can provide these values through command line arguments.

    To run Annotator on the target project P, the arguments below must be passed to Annotator:

    Flag Description
    -bc,--build-command <arg> Command to run NullAway on target P enclosed in "". Please note that this command should be executable from any directory (e.g., "cd /Absolute/Path/To/P && ./build").
    -i,--initializer <arg> Fully qualified name of the @Initializer annotation.
    -d,--dir <arg> Directory where all outputs of AnnotatorScanner and NullAway are serialized.
    -cp, --config-paths Path to a TSV file containing values defined in Error Prone config paths given in the format: (path_to_nullaway_config.xml \t path_to_scanner_config).
    -cn, --checker-name Checker name to be used for the analysis. (use NULLAWAY to request inference for NullAway.)

By default, Annotator has the configuration below:

  1. When a tree of fixes is marked as useful, it only injects the root fix.
  2. Annotator will bailout from the search tree as soon as its effectiveness hits zero or less.
  3. Performs search to depth level 5.
  4. Uses javax.annotation.Nullable as @Nullable annotation.
  5. Cache reports results and uses them in the next cycles.
  6. Parallel processing of fixes is enabled.
  7. Downstream dependency analysis is disabled.

Here are some useful optional arguments that can alter the default configurations mentioned above:

Flag Description
-n,--nullable <arg> Sets custom @Nullable annotation.
-rboserr, --redirect-build-output-stderr Redirects build outputs to STD Err.

To learn more about all the optional arguments, please refer to OPTIONS.md

Here is a template command you can use to run Annotator from the CLI, using CLI options-

java -jar ./path/to/annotator-core.jar -d "/path/to/output/directory" -cp "/path/to/config/paths.tsv" -i com.example.Initializer -bc "cd /path/to/targetProject && ./gradlew build -x test"

To view descriptions of all flags, simply run the JAR with the --help option.

NullAway Compatibility

  • Annotator version 1.3.6 is compatible with NullAway version 0.10.10 and above.

License

Annotator is released under the MIT License. See the LICENSE file for more information.

nullawayannotator's People

Contributors

lazaroclapp avatar msridhar avatar nimakarimipour avatar pjsrcool avatar raghuganapathyucr avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar

nullawayannotator's Issues

Upgrade to javaparser v3.24.4

Describe the task
Upgrade to javaparser 3.24.4.

Expected behavior
Use javaparser 3.24.4.

Additional context
Not-Applicable.

Build requests from Annotator may not execute due to build systems caching strategy.

Describe the bug
In environments where the build system is extensively using caching strategies, a build request from Annotator may not execute if no file is modified under the module directory. Even if config files of annotator are written inside the module, since config files content can be identical within build request, it might hit the cache and the using build system will not rebuild the module. If the project is not rebuilt, annotator will not correctly compute the effectiveness of fixes and the serialization outputs may not be even located.

To Reproduce
Re-use the AutoAnnotator

Expected behavior
When ever annotator is requesting a build, all Error Prone checkers should re-execute.

Screenshots
Not-Applicable.

OS (please complete the following information):

  • OS: Linux
  • Annotator Version: v1.3.2-SNAPSHOT

Additional context
Not-Applicable.

Activate downstream dependency test in CI

Describe the task
Currently DownstreamDependencyTest is excluded in the CI since it requires NullAway:0.9.9 to be released. This test can only pass locally with NullAway:0.9.9-SNAPSHOT. Include downstream dependency test in the CI once NullAway:0.9.9 is released.

Expected behavior
Downstream dependency tests must also be included in the CI test.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Remove hard code values of NullAway/Scanner versions in Unit tests.

Describe the task
NullAway/Type Annotator Scanner versions are hardcoded in unit tests and every change in these numbers, requires modifying multiple files. This structure is extremely prone to error in updating all files.

Expected behavior
All test configurations must be updated according to a unique source.

Additional context
Not-Applicable

Extend global analysis to cover fields/arguments and protected methods

Describe the task
Currently we only track the effects of changes on public method returns on downstream dependencies. We can extend it to fields/arguments and protected methods on downstream dependencies as well.

Expected behavior
Consider the effects of applying changes on fields and arguments on downstream dependencies.

Additional context
Add any other context about the task here.

Root fix injection mode misses impacted paramaters

Describe the bug
If the mode for applying reports is Root Fix only, it will miss injections of @Nullable annotations on impacted parameters.

To Reproduce
Have method the flows nullability back to upstream in downstream dependency and have the mode set to Root Fix instead Chain fix.

Expected behavior
Annotate impacted parameters in all modes.

Stack trace
Not-Applicable.

OS (please complete the following information):

  • Annotator Version [e.g. v1.3.3-SNAPSHOT]

Additional context
Not-Applicable.

Crash: ConcurrentModificationError

Describe the bug
In environment where multiple instances of Scanner is running, it is possible that the tool crashes in error prone checks because of mutable static fields. The exception is ConcurrentModificationError

To Reproduce
Run multiple instance of Scanner in parallel.

Expected behavior
Stable run of multiple Scanner instances.

Stack trace

java.util.ConcurrentModificationException
  	at java.base/java.util.HashMap$KeySpliterator.tryAdvance(HashMap.java:1644)
  	at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:127)
  	at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:502)
  	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:488)
  	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
  	at java.base/java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:150)
  	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
  	at java.base/java.util.stream.ReferencePipeline.findAny(ReferencePipeline.java:548)
  	at edu.ucr.cs.riple.scanner.out.MethodInfo.findOrCreate(MethodInfo.java:65)
  	at edu.ucr.cs.riple.scanner.out.MethodInfo.findParent(MethodInfo.java:89)
  	at edu.ucr.cs.riple.scanner.Scanner.matchMethod(Scanner.java:106)
  	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:741)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitMethod(ErrorProneScanner.java:152)
        ......

OS (please complete the following information):

  • OS: OS X
  • Version 12.5
  • Annotator Version: v1.3.2-SNAPSHOT

Additional context
Not-Applicable.

Treat impacted parameters as triggered fixes.

Describe the task
At this moment we treat impacted parameters as a result of making methods @Nullable in upstream and receiving the via downstream dependencies as below:

Assumption: If method m is selected to be @Nullable , parameters P1, P2, .... will receive @Nullable due to usage in downstream dependencies.

After injection on m, Annotator will process making P1, P2, .... @Nullable individually in the next iteration and will mark each as @Nullable if it reduces the number of errors. (Not all may reduce the number of errors).

in the suggested approach Annotator should treat P1, P2, .... as triggered fixes and process them in the same iteration of processing m.

Expected behavior
Treat list of P1, P2, .... as triggered fixes for m in the same iteration of processing m and include them in the fix tree. If the total effect is more than 0, discard the whole fix tree.

Additional context
This is strictly in favor of resolving #62.

Rename Scanner module to NullAwayAnnotatorScanner

Describe the task
As @lazaroclapp suggested, the name Scanner is too generic, specifically when mentioned along other ErrorProneCheckers and should be refined. Currently the suggestion is: NullAwayAnnotatorScanner.

Expected behavior
Rename of module Scanner to NullAwayAnnotatorScanner with follow ups of all required updates.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Update Scanner serialization control flags.

Describe the task
Currently Scanner has multiple flags that control the serialization at fine grained level which is not required. All serialization can be controlled via a single flag.

Expected behavior
Remove existing control flags for serialization and only use one global flag which controls all serializations for Scanner.

Remove redundant conflicts from Lombok generated code.

Following #118 we had significant increase on the number of conflicts due to Lombok generated code. Looking at generated code by Lombok, It generates three methods equals(), hashcode() and toString() which want to operate on all class fields in their body. This means that if the class has n fields, it will require at least n builds while it is impossible for these three methods to have an NPE on the method body or at the call sites (Their body is written with null-checks). Looking more into lombok generated code, it seems if a field is @Nullable, all code using that field is written with null-checks. It seems to me that regions created by lombok are free of NPEs and we can exclude usages in those regions. This will have a significant impact on reduction of conflicts.

The reason that we are seeing the updated version having more conflicts than previous one is that in the body of equals(), hashCode() and toString() Lombok, instead of using direct reference to fields, it uses getter methods for that fields, and this causes a lot of conflicts.

Parallel processing mode misses impacted region for initialization error.

Describe the bug
Following #127 which runs all tests in all configurations, noticed a bug when Parallel Processing mode is activated compared to not activated. In #658, for a @Nonnull field that is not initialized at declaration and the constructor fails to initialize it, the region member for the enclosing region is null. We currently do not watch these regions in parallel processing mode and a reported error might be missed. See example below:

class Foo {
    Object bar;
    Foo() {
         this.bar = initBar();
    }
    @Nullable Object initBar() { return null; }
}

Currently the potentially impacted regions set is only {Foo#Foo}, but we will see an extra error in region Foo#null which will be missed in parallel processing mode since Foo#null is not included in the potentially impacted regions.

Expected behavior
Include null region for methods that are called in constructors.

Additional context
In NullAway, the visitor state path is pointing to the enclosing class tree when reporting METHOD_NO_INIT errors, and that is the reason we are seeing null getting serialized as the region member (class tree is not enclosed by any member of the enclosing class).

Update README.md

Describe the task
Annotator has changed a lot in the new upcoming version. README.md files are missing config flags.

Expected behavior
Update README.md.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Collect and print stdout output from command that builds downstream dependencies

This is the code that runs the command for building downstream dependencies:

public static void executeCommand(Config config, String command) {
try {
Process p = Runtime.getRuntime().exec(new String[] {"/bin/sh", "-c", command});
BufferedReader reader =
new BufferedReader(new InputStreamReader(p.getErrorStream(), Charset.defaultCharset()));
String line;
while ((line = reader.readLine()) != null) {
if (config.redirectBuildOutputToStdErr) {
System.err.println(line);
}
}
p.waitFor();
} catch (Exception e) {
throw new RuntimeException("Exception happened in executing command: " + command, e);
}
}

It collects the stderr output from the command, but not the stdout. It would be good to have a way to collect and print stdout as well, for diagnosing issues. Also, the current logic looks a bit weird. There is a while loop that spins collecting the stderr output. I think this loop should run until the process terminates (otherwise you may miss some output). But if it runs until the process terminates, then I think the p.waitFor() call is unnecessary? We may just want to switch to code like this:

https://stackoverflow.com/a/12200361/1126796

With Redirect.INHERIT we will get the printing of stdout / stderr for free. (Or possibly we want to redirect stderr of the child process to stdout, and then just send everything to our stdout.)

Injector language level is not configurable

Describe the bug

Language level of javaparser by default does not support recent changes in JDK 17. It should be configured via ParserConfiguration instance.

ParserConfiguration conf = new ParserConfiguration();
conf.setLanguageLevel(ParserConfiguration.LanguageLevel.JAVA_17);
StaticJavaParser.setConfiguration(conf);
StaticJavaParser.parse(file);

To Reproduce

call StaticJavaParser.call() on a source file containing expression (o instanceof Foo bar).

Expected behavior
Parse source file with no ParseException. And setting the language levels below:

Stack trace
Not-Applicable.

OS (please complete the following information):

  • OS: OS X
  • Annotator Version: v1.3.4-SNAPSHOT

Additional context
This issue is not applicable if we migrate from using javaparser to google-java-format.

Update to upcoming new release of NullAway 0.9.10

Describe the task
Update Annotator to work with the upcoming release of NullAway. In the new release of NullAway (PR643), information of the non-null target (the element involved in the pseudo-assignment of a @nullable expression into a @nonnull and causing the error) is serialized along the reporting errors in the errors.tsv. This requires the parsing methods to be updated.

Expected behavior
Work as expected with NullAway 0.9.10.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Rerun analysis at the end of the process.

Describe the task
Annotator caches the result of explorations and will reuse them in all further iterations. If a fix tree is not reducing the number of errors, it will be tagged as an unuseful branch and annotator will never investigate its effect in further iterations. However, at the end of the process, the state of the program has altered due to injection of series of annotations in previous iterations and those tagged branches can be potentially usefull and reduce the number of errors further. Annotator should invalidate the branches tags at the end of the process and rerun the analysis for one more iteration.

Expected behavior
Invalidate reports cache and rerun analysis for only one more iteration.

Additional context
Not-Applicable.

Prevent build failures on downstream dependencies due to changes in upstream

Is your feature request related to a problem? Please describe.

If annotator processes the target module with --downstream-dependency-analysis-activated flag, it will collects effects of making public methods @Nullable on downstream dependencies and considers the overall effect while making inference decisions. Some annotations can reduce the number of errors locally far more than the number of triggered errors on downstream dependencies and can have a positive overall impact, hence, these annotations will get injected to the target module (even though this will lead to build errors on downstream dependencies).

It is a common use case where the user would like to run the AutoAnnotator on target module with following constraints:

  • No modification on downstream dependencies is allowed.
  • No build failures on downstream dependencies after enrolling the target module.

Describe the solution you'd like

A configuration mode which if enabled, it guarantees that no changes in upstream will trigger an error on downstream dependencies. This should also cover detection of flow of @Nullable back to upstream. See example below:

Target Module:

class Foo{

     public Object returnNullBad(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     public Object returnNullGood(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     public void takeNullGood(Object param){ // Called only in downstream dependencies.
              if(param != null){
                     param.deref();
              }
     }

     public void takeNullBad(Object){ // Called only in downstream dependencies.
             param.deref();
     }
}

Downstream Dependency Module:

class Dep{

     Object field = new Object();
     Foo foo = new Foo();
     
     public void bar(){
             field = foo.returnNullBad();
             foo.takeNullGood(foo.returnNullBad());
             foo.takeNullGood(foo.returnNullGood());
             foo.takeNullBad(foo.returnNullGood());
     }
}

If Annotator only focus on the overall impact, it will annotate the Target module as below:

class Foo{

     @Nullable   // This will resolve 5 errors locally and triggers 1 unresolvable and 1 resolvable error (on Dep) overall: -4
     public Object returnNullBad(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     @Nullable // This will resolve 5 errors locally and 1 resolvable error (change on Target) overall: -5
     public Object returnNullGood(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     // This will resolve 1 error on downstream dependency and create no error locally
     public void takeNullGood(@Nullable Object param){ // Called only in downstream dependencies.
              if(param != null){
                     param.deref();
              }
     }

     // Since @Nullable injection will create an error locally and the overall is +1,
     // it will not get injected, however, this will leave the error on downstream unresolved.
     public void takeNullBad(Object){ // Called only in downstream dependencies.
             param.deref();
     }
}

With the requested configuration mode, annotator should annotate Target module as below:

class Foo{

     @NullUnmarked //To resolve the error on downstream dependency.
     public Object returnNullBad(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     @Nullable
     public Object returnNullGood(){
              // Assume there are 5 branches ending with return null;
              return null;
     }

     public void takeNullGood(@Nullable Object param){ // Called only in downstream dependencies.
              if(param != null){
                     param.deref();
              }
     }

     @NullUnmarked //To resolve the error on downstream dependency.
     public void takeNullBad(Object){ // Called only in downstream dependencies.
             param.deref();
     }
}

Describe alternatives you've considered
None.

Additional context
Not-Applicable.

Repeated annotations and broken formatting when adding suppressions

Describe the bug
When running the auto-annotator on suppression (and @NullUnmarked) adding mode, I am observing two issues on fields that otherwise already contain annotations:

  • Duplicated instances of @Nullable added
  • Lack of spaces between the last added annotation and the field's modifiers

For example, this is the initial code:

@JsonProperty
@JsonInclude(JsonInclude.Include.NON_NULL)
public String foo;

And this the auto-annotator's result (before Google Java Format, but that's irrelevant for now because this code won't build):

@JsonProperty @JsonInclude(JsonInclude.Include.NON_NULL) @Nullable @SuppressWarnings("NullAway.Init")
@Nullablepublic   String foo;

Note: a) @Nullable is repeated, despite it already having @SuppressWarnings("NullAway.Init") (which makes no sense combined with @Nullable); b) @Nullablepublic as opposed to @Nullable public

To Reproduce
See code-example above. Configuration includes --chain --activate-downstream-dependencies-analysis --force-resolve.

Expected behavior
I would expect that field to either be annotated:

@JsonProperty
@JsonInclude(JsonInclude.Include.NON_NULL)
@SuppressWarnings("NullAway.Init")
public String foo;

Or:

@JsonProperty
@JsonInclude(JsonInclude.Include.NON_NULL)
@Nullable
public String foo;

Stack trace
N/A

OS (please complete the following information):

  • OS: Linux
  • Annotator Version : this commit, currently the head of #76

Use Gradle command line arguments to setup test configurations.

Describe the task
Currently project template build..gradle config files, are rewritten inside the java test classes with the values computed at runtime. A better solutions is to pass the computed values via gradle command line arguments, this way, we do not need to rewrite build.gradle files.

Expected behavior
Pass the value of test directory path via gradle command line arguments.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Force resolve cannot suppress errors for nullable param pass in field initializations.

Describe the bug
We currently support adding SuppressWarnings and NullUnmarked annotation to silence remaining errors through #69. After the outer loop completes, Annotator will run NullAway another time and surpasses all remaining errors with either @NullUnmarked or @SuppressWarnings. All remaining errors store their corresponding regions where that error is triggered. For all triggered regions where the enclosing method is not "null", we mark them with @NullUnmarked annotation. Remaining errors are triggered in field declaration regions of classes. For field declarations, if they are assigned to @Nullable we suppress it by adding @SupreessWarnings("NullAway") and if they are nit initialized we add @SupreessWarnings("NullAway.Init"). This does not cover all cases, such as example below:

class Foo {
     Bar f = new Bar(null);
}

In this case we will receive the error: passing @Nullable where @Nonnull was expected and the triggered regions are class: Foo method: "null". This case is not covered in our current strategy. We should mark the called constructor with @NullUnmarked.

To Reproduce
Run with force resolve mode activated on the code below:

class Foo {
     Bar f = new Bar(null);
}

Expected behavior
Mark Bar(Object) as @NullUnmarked.

Stack trace
Not-Applicable.

OS (please complete the following information):

  • OS: x.
  • Version: x.
  • Annotator Version: v1.3.3-SNAPSHOT.

Additional context
Not-Applicable.

Install Core jar in Maven Local.

Is your feature request related to a problem? Please describe.
We can create the jar for core module via command ./gradlew shadowJar, however, this file will be created in build/libs/core.jar. Most users will need to access this in maven local in the correct path according to maven publish info.

Describe the solution you'd like
A single task which recreates the core jar and installs it in the maven local.

Describe alternatives you've considered
Not-Applicable.

Additional context
Not-Applicable.

Remove size serialization of method parameters.

Describe the task
Remove number of arguments serialization from Scanner checker.

Expected behavior
Calculate the number of required arguments from the method signature itself.

Screenshots
Not-Applicable.

Additional context
No additional context available.

Update Uber Copy Right Headers

Describe the task
Update all Uber Copy Right Headers to a full version where a code is copied from NullAway, not just the title.

Expected behavior
Not-Applicable.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Remove serialization of parameter names / URI for methods.

Describe the task
List of parameter names / URIs are not required any more by the core module due to the enhancement of injector module, and should be removed.

Expected behavior
Serialization of methods by scanner module should contain method parameters names / URIs.

Screenshots
Not-Applicable.

Additional context
Not-Applicable.

Update readme.md to show status of CI

Describe the task
Update the readme.md to show status of CI jobs.

Expected behavior
Status of CI jobs shown in the landing readme.md.

Additional context
Not-Applicable.

Scanner Checker Fails when not activated.

Describe the bug
Scanner checker throws null-pointer-exception when the checker is not activated.

To Reproduce
Steps to reproduce the behavior:

  1. Build the project when the config file is not passed as error prone flag.

Expected behavior
When the config file is not passed, the checker should not terminate with exception, instead it should silently quit from every method.

Redesign test architecture

Describe the task

The current CI is rather a heavy process due to the unit tests architecture. Each unit test is converted to a standalone gradle project and that makes each unit test somewhat heavy. We currently have 53 unit tests and even though the procedure is parallelized ( to the number of cores), in my laptop with 8 cores it takes more than 10 minutes. We are planning to have a CI that runs all tests with all optimization disabled and that should take even more.

TODO: Add Precondition

Describe the task
Add the precondition at here one a NullAway version 0.9.9 is released.

Expected behavior
Enables us to inform us once a new type of annotation is present in the input.

Screenshots
Not applicable.

Additional context
Not availble.

Augment error serialization to enable distinguishing each one individually.

Describe the task
Currently the information below are serialized for each error in target module.

message_type    message    enc_class    enc_method    kind    class   method     param    index    uri    

With the information below if we have the code snippet below:

void foo(){
      bar(nul);
      bar(nul);
      bar(nul);
}

We will see four identical lines in errors.tsv corresponding to each error. Due to this limitation, in every collection of errors, we have to use subtypes of List as they allow duplicates. However, using Set can be a better structure as it will inform us if we mistakenly add an error multiple times.

Update Annotator to creates different instances of Error for each error.

Expected behavior
Creating different instances of Error for the shown code snippet and convert each collection of errors from List to Set.

Additional context
Before Annotator can make this distinguish, NullAway must serialize information regarding each error (e.g. source location).

Injector misses enums constants while other anonymous classes exists

Describe the bug
According to java language specifics, Enum constants are first members of enums. In javaparser structure, enum constants are stored after all other members of enum which does not conform to how javac assigns flat names. We should first prioritize locating enums constants over other anonymous classes. As shown in the example below, enum constants are numbered before other existing anonymous classes, but in javaparser data structures, in the list of child nodes, enum constants are the last elements.

This will cause injector to annotate a wrong method e.g. EC2#bar(), instead of Anonymous#bar() or miss the annotation.

public enum Main {

    EC1 { --------------------------------//Main$1.
        @Override
        public String bar() {
            return "EC1 IMPL";
        }
    },

    EC2 { --------------------------------//Main$2.
        @Override
        public String bar() {
            return "EC2 IMPL";
        }
    };

    A a = new A() {-----------------------//Main$3.
        @Override
        public String bar() {
            return null;
        }
    };

    public final void foo() {
        System.out.println("TEST");
    }

    public abstract String bar();
}

interface A {
    String bar();
}

Support @NullUnmarked

Describe the task
Add a configuration mode that if enabled, at the end of the process, annotator marks the closest enclosing method of the remaining errors with @NullUnmarked annotation to guarantee NullAway checks pass.

Expected behavior
All remaining errors should be enclosed by the @NullUnmarked annotation.

Additional context
@NullUnmarked can be located here.

Modify injector architecture

Describe the task
It seems that javaparser is robust at parsing java source code. Preserving the code style is really important for our use case, considering that we had some issues with annotation insertion / deletion we might be able to utilize javaparser in computing the offsets where the @Nullable annotation should be added and then directly modify the content of the containing source file.

Expected behavior
All the annotations get inserted without breaking anything in the code. (modifier removal, syntax errors or changing the code style).

Additional context
With this issue closed, #77 and #78 can also be closed.

Wrong uri in MethodDeclarationTree nodes

Describe the bug
For TypeAnnotatorScanner

URIs are fetched from the VisitorState instances. For sub methods, the URI is fetched from a state while it is pointing at the super method and a wrong URI is fetched.

To Reproduce
Run TypeAnnotatorScanner on modules with base and super class.

Expected behavior
All nodes in the tree have the uri to the containing source file.

Stack trace
Not-Applicable

OS (please complete the following information):

  • Annotator Version [e.g. v1.3.4-SNAPSHOT # master]

Additional context
This will also prevent Annotator to correctly compute annotations for @NullUnmarked injections.

Annotator fails when a Fix targeting an element in a generated code is suggested.

Describe the bug
In the process of measuring effectiveness of fixes, if a fix is targeting an element in the generated code, Injector will throw an exception that it could not find the class which does not exist.

To Reproduce
Use a lombok builder and target a method in the builder class.

Expected behavior
Injector should report these classes silently when the class is not found and should not terminate the process.

Screenshots
Not-Applicable.

Additional context
No Additional Context.

Incorrect computation of lower bound of number of errors on downstream dependencies in fix trees.

Describe the bug
At the current state, Annotator only considers the effect of root of the fix tree on downstream dependencies when analyzing the effect of a fix tree on downstream dependencies. This lower bound is not sufficient for Annotator to correctly make decisions and it can be much lower than the actual computable lower bound. This can lead to injecting @Nullable on public methods that introduces a lot of errors as a result of inferior lower bound computation.

To Reproduce
Consider a fix tree with root fix method:bar that has method:foo in its chain and bar and foo leads to 0 and 100 errors on downstream dependencies if annotated as @Nullable. At this state the lower bound computation will only consider the root tree which is 0 and is wrong. The actual lower bound is 100.

Expected behavior
When computing the lower bound of the number of errors in a fix tree, return the maximum number of errors each method can make on downstream dependencies.

Screenshots
Not-Applicable.

OS (please complete the following information):

  • OS: OS X
  • Version 12.5
  • Annotator Version: v1.3.3-SNAPSHOT

Additional context
Not-Applicable.

Crash by Precondition - Missing triggered fixes from downstream

Describe the bug
Annotator incorrectly considers some fixes to be safe to inject on target. It currently considers fixes that introduce new errors on downstream dependencies that can be resolved via a separate fix on target, safe. It fails to also ensure that the resolving fix is also included in the fix tree.

To Reproduce

@Test
  public void upperBoundCountForResolvableErrorOnDownstreamTest() {
    coreTestHelper
        .onTarget()
        .withSourceLines(
            "Bar.java",
            "package test;",
            "public class Bar {",
            "   public String foo;",
            "   public String foo2;",
            "   public void setFoo(String foo) {",
            "     this.foo = foo;",
            "   }",
            "   public String getFoo() {",
            "     return foo;",
            "   }",
            "}")
        .withDependency("Dep")
        .withSourceLines(
            "Dep.java",
            "package test.dep;",
            "import test.Bar;",
            "public class Dep {",
            "   public Bar bar = new Bar();",
            "   public void exec() {",
            "     bar.foo2 = bar.getFoo();",
            "   }",
            "}")
        .withExpectedReports()
        .disableBailOut()
        .enableDownstreamDependencyAnalysis(AnalysisMode.STRICT)
        .toDepth(5)
        .start();
  }

Expected behavior
Even though errors due to fix on getFoo() is resolvable on downstream, the corresponding fix is not included in the fix tree, hence, Annotator should consider this fix as unsafe.

Stack trace

java.lang.IllegalArgumentException
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:131)
	at edu.ucr.cs.riple.core.AnalysisMode$2.lambda$tag$0(AnalysisMode.java:69)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at edu.ucr.cs.riple.core.AnalysisMode$2.tag(AnalysisMode.java:61)
	at edu.ucr.cs.riple.core.Annotator.executeNextIteration(Annotator.java:150)
	at edu.ucr.cs.riple.core.Annotator.annotate(Annotator.java:110)
	at edu.ucr.cs.riple.core.Annotator.start(Annotator.java:72)
...

Please complete the following information:

  • Annotator Version: 1.3.9-SNAPSHOT

Additional context
None

Crash on retrieving undiscovered method node.

Describe the bug
location for method can be null. The reason is for dereferencing either location for TOP method node or an unseen method.
The top method node is indexed at -1 while the reserved id is 0. Due to this mismatch, the TOP node was retirvied from the list and not identified as the TOP node.

Also getClosesSuperMethod() returns the methodNode even if the superMethod is not declared in the module.

To Reproduce

Expected behavior
TOP node with id 0 should be at index 0 and only method nodes for seen methods must be returned.

Stack trace

Analyzing at level 1, Exception in thread "main" java.lang.NullPointerException
        at edu.ucr.cs.riple.core.metadata.trackers.MethodRegionTracker.getRegions(MethodRegionTracker.java:77)
        at edu.ucr.cs.riple.core.metadata.trackers.CompoundTracker.getRegions(CompoundTracker.java:61)
        at edu.ucr.cs.riple.core.metadata.graph.Node.lambda$reCollectPotentiallyImpactedRegions$1(Node.java:119)
        at java.base/java.lang.Iterable.forEach(Iterable.java:75)
        at edu.ucr.cs.riple.core.metadata.graph.Node.reCollectPotentiallyImpactedRegions(Node.java:119)
        at edu.ucr.cs.riple.core.explorers.OptimizedExplorer.lambda$initializeFixGraph$0(OptimizedExplorer.java:59)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.lambda$forEachRemaining$1(CollectSpliterators.java:377)
        at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1675)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.forEachRemaining(CollectSpliterators.java:373)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
        at edu.ucr.cs.riple.core.explorers.OptimizedExplorer.initializeFixGraph(OptimizedExplorer.java:59)
        at edu.ucr.cs.riple.core.explorers.Explorer.explore(Explorer.java:102)
        at edu.ucr.cs.riple.core.Annotator.processTriggeredFixes(Annotator.java:227)
        at edu.ucr.cs.riple.core.Annotator.executeNextIteration(Annotator.java:170)
        at edu.ucr.cs.riple.core.Annotator.annotate(Annotator.java:138)
        at edu.ucr.cs.riple.core.Annotator.start(Annotator.java:82)
        at edu.ucr.cs.riple.core.Main.main(Main.java:46)

OS (please complete the following information):

  • Annotator Version [e.g. v1.3.3-SNAPSHOT]

Additional context
Not-Applicable.

Annotations are mixed with modifier

Describe the bug
Added annotations are getting mixed with the modifier.

-  @JsonProperty public List<String> bar;
+  @JsonPropertypublic @Nullable  List<String> bar;

Or

-  @JsonProperty
-  @JsonInclude(JsonInclude.Include.NON_NULL)
-  public String foo;
+  @JsonProperty @JsonInclude(JsonInclude.Include.NON_NULL) @Nullable
+@Nullablepublic   String foo;

To Reproduce

  • The root cause is not detected yet.

Expected behavior

-  @JsonProperty
-  @JsonInclude(JsonInclude.Include.NON_NULL)
-  public String foo;
+  @JsonProperty @JsonInclude(JsonInclude.Include.NON_NULL) @Nullable
+  public   String foo;

and

-  @JsonProperty public List<String> bar;
+  @JsonProperty public @Nullable  List<String> bar;

Stack trace
Not-Applicable.

OS (please complete the following information):

  • Annotator Version [e.g. v1.3.3-SNAPSHOT]

Additional context
This is a subproblem of #77

Use @NullUnmarked in JSpecify in released version 0.3.0 for unit tests

Describe the task
JSpecify version 0.3.0 is released and according to their documentation, it is very unlikely that the containing annotations in version 0.3.0 gets changed in any incompatible way. It worths cleaning up the qual module which only contains the @NullUnmarked annotation provided for tests and reuse org.jspecify:jspecify:0.3.0 in tests configurations.

Expected behavior
Remove qual module and use org.jspecify:jspecify:0.3.0 in test configurations.

NPE crash on auto-annotator v1.3.6 in the presence of Lombok generated constructor

I am getting an auto-annotator error due to Lombok.

In code like:

  @AllArgsConstructor
  @EqualsAndHashCode
  public static class Foo {

    private final String a;
    private final String b;
    private final String c;

    public Foo(String a, String b) {
      this.a = a;
      this.b = b;
      this.c = null;
    }

    public Foo(String c) {
      this.a = null;
      this.b = null;
      this.c = c;
    }
  }

I get the following NPE crash:

Exception in thread "main" java.lang.NullPointerException
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$1(DownstreamImpactEvaluator.java:97)
        at java.base/java.lang.Iterable.forEach(Iterable.java:75)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$2(DownstreamImpactEvaluator.java:88)
        at edu.ucr.cs.riple.injector.location.OnMethod.ifMethod(OnMethod.java:96)
        at edu.ucr.cs.riple.core.metadata.index.Fix.ifOnMethod(Fix.java:99)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.lambda$collectGraphResults$3(DownstreamImpactEvaluator.java:70)
        at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1655)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.lambda$forEachRemaining$1(CollectSpliterators.java:377)
        at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1693)
        at com.google.common.collect.CollectSpliterators$FlatMapSpliterator.forEachRemaining(CollectSpliterators.java:373)
        at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactEvaluator.collectGraphResults(DownstreamImpactEvaluator.java:68)
        at edu.ucr.cs.riple.core.evaluators.AbstractEvaluator.evaluate(AbstractEvaluator.java:96)
        at edu.ucr.cs.riple.core.cache.downstream.DownstreamImpactCacheImpl.analyzeDownstreamDependencies(DownstreamImpactCacheImpl.java:114)
        at edu.ucr.cs.riple.core.Annotator.annotate(Annotator.java:134)
        at edu.ucr.cs.riple.core.Annotator.start(Annotator.java:84)
        at edu.ucr.cs.riple.core.Main.main(Main.java:46)

Which some debugging prints trace to the three argument constructor automatically injected by the lombok @AllArgsConstructor annotation (i.e. Foo(String, String, String) which isn't present in the source code, but which lombok generates). I thought we had handling in place for methods being missing from the source?

Deactivate inference.

Describe the task
For experimenting purposes, it worths to measure the effectiveness of inference of the Annotator in reducing the required total number of @NullUnmarked to resolve all errors. To do that, a special mode is desired where it deactivates the inference procedure (no @Nullable injection). With this mode we can compare the total number of @NullUnmarked injections with inference activated \ deactivated.

Expected behavior
A flag to disable inference and resolve all errors with @NullUnmarked injections.

Additional context
This feature is useful for experiments purposes only.

Annotator is unaware of annotations copied by Lombok in Strict Mode

Describe the bug
In strict mode, Annotator should not make any changes on the target module, that may introduce new errors on downstream dependencies. Although Annotator is aware of possible errors in downstream methods for a generated getter by Lombok (getFoo()), but it is unaware of copying mechanism by Lombok that annotations on foo will get copied on getFoo as well. Hence, it might introduce an error in downstream dependencies.

Expected behavior
Annotator should be aware of annotations getting copied on generated getter methods for fields by Lombok. And reject a tree, if any of their getter is creating new errors in downstream dependencies.

Stack trace
Not-Applicable.

OS (please complete the following information):

  • Annotator Version [1.3.6]

Additional context
Not-Applicable.

Wrong serialization of @NotNull formal parameters with type use annotation

Describe the bug
Scanner serializes a wrong method signature for @NotNull formal parameters annotated with a type use annotation. The type use annotation fully qualified name is included in the serialized method signature which should not.

To Reproduce
Run scanner for the code below:

package test;
import org.jetbrains.annotations.NotNull;
public class A {
  static void foo(@NotNull Object o) {}
  static void bar() { foo(null); }
}

Scanner serializes foo(@org.jetbrains.annotations.NotNull java.lang.Object) as the method signature for o.

Expected behavior
It should just serialize foo(java.lang.Object).

Stack trace
Not applicable.

OS (please complete the following information):

  • Annotator version : 1.3.10-SNAPSHOT

Add informative comment on @NullUnmarked injections

Is your feature request related to a problem? Please describe.

It is not clear to the programmer why the element is annotated as @NullUnmarkd instead of @Nullable if the impact on downstream dependencies prevented annotator from adding @Nullable.

Describe the solution you'd like

Add informative comments on @NullUnmarked injections describing the impact of the annotated method locally and globally. With the format below as suggested by @msridhar:

// local for errors within the scope of the annotation, //non-local for errors outside annotation scope but within the same module, and //dependent module for errors in dependent modules

Describe alternatives you've considered
None.

Additional context
None.

Use `@Contract` instead of `@NullUnmarked`

Is your feature request related to a problem? Please describe.
Sometimes it is quiet easy to figure out when @Contract("null -> null") should be added at the method declaration itself. Currently, autoannotator adds @Nullunmarked on the invocations.

Describe the solution you'd like
Add @Contract

cc: @lazaroclapp

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.