Conversation
Stacktrace from version 23.1.0 : java.lang.NullPointerException: Cannot invoke "java.util.List.contains(Object)" because "validExtensionsWithDot" is null at org.approvaltests.reporters.GenericDiffReporter.isFileExtensionValid(GenericDiffReporter.java:122) at org.approvaltests.reporters.GenericDiffReporter.isFileExtensionHandled(GenericDiffReporter.java:117) at org.approvaltests.reporters.GenericDiffReporter.isWorkingInThisEnvironment(GenericDiffReporter.java:104) at org.approvaltests.reporters.GenericDiffReporter.report(GenericDiffReporter.java:56) at org.approvaltests.reporters.FirstWorkingReporter.report(FirstWorkingReporter.java:21) at org.approvaltests.reporters.FirstWorkingReporter.report(FirstWorkingReporter.java:21) at org.approvaltests.reporters.FirstWorkingReporter.report(FirstWorkingReporter.java:21) at org.approvaltests.reporters.FirstWorkingReporter.report(FirstWorkingReporter.java:21) at org.approvaltests.approvers.FileApprover.reportFailure(FileApprover.java:49) at org.approvaltests.Approvals.verify(Approvals.java:220) at org.approvaltests.Approvals.verify(Approvals.java:183) at org.approvaltests.Approvals.verify(Approvals.java:191) at org.approvaltests.Approvals.verify(Approvals.java:50)
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes the GenericDiffReporter constructor so the provided valid extension list is stored on the instance, preventing the reported NullPointerException when checking whether a file extension is supported. Sequence diagram for GenericDiffReporter extension validationsequenceDiagram
Approvals->>GenericDiffReporter: report()
GenericDiffReporter->>GenericDiffReporter: isWorkingInThisEnvironment()
GenericDiffReporter->>GenericDiffReporter: isFileExtensionHandled()
GenericDiffReporter->>GenericDiffReporter: isFileExtensionValid()
GenericDiffReporter-->>GenericDiffReporter: validExtensions.contains(extension)
GenericDiffReporter-->>Approvals: extension support result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="approvaltests/src/main/java/org/approvaltests/reporters/GenericDiffReporter.java" line_range="50" />
<code_context>
this.arguments = argumentsFormat;
this.diffProgramNotFoundMessage = diffProgramNotFoundMessage;
- validExtensions = validFileExtensions;
+ this.validExtensions = validFileExtensions;
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The change is behaviorally identical to the removed assignment and does not prevent `validExtensions` from remaining null. When `validFileExtensions` is null—as permitted by the four-argument constructor and used by existing tests—`isFileExtensionHandled` passes null to `isFileExtensionValid`, whose `contains` call raises the reported NullPointerException.
**Triggers:** When a reporter is constructed with a null extension list.
**Suggested fix:** Default null extension lists to an empty list or handle null in `isFileExtensionHandled`/`isFileExtensionValid`.
```suggestion
this.validExtensions = validFileExtensions == null ? new ArrayList<String>() : validFileExtensions;
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: approvaltests/src/main/java/org/approvaltests/reporters/GenericDiffReporter.java:50
| this.arguments = argumentsFormat; | ||
| this.diffProgramNotFoundMessage = diffProgramNotFoundMessage; | ||
| validExtensions = validFileExtensions; | ||
| this.validExtensions = validFileExtensions; |
There was a problem hiding this comment.
issue (bug_risk): The change is behaviorally identical to the removed assignment and does not prevent validExtensions from remaining null. When validFileExtensions is null—as permitted by the four-argument constructor and used by existing tests—isFileExtensionHandled passes null to isFileExtensionValid, whose contains call raises the reported NullPointerException.
Triggers: When a reporter is constructed with a null extension list.
Suggested fix: Default null extension lists to an empty list or handle null in isFileExtensionHandled/isFileExtensionValid.
| this.validExtensions = validFileExtensions; | |
| this.validExtensions = validFileExtensions == null ? new ArrayList<String>() : validFileExtensions; |
Stacktrace from version 23.1.0 :
Summary by Sourcery
Bug Fixes: