What the linter reports
Go's nilness static analyser flags a tautological nil check in HaveAppStudioTestsFinished() in gitops/snapshot.go:
tautological condition: non-nil != nil nilness [Ln 709, Col 25]
Current code
func HaveAppStudioTestsFinished(snapshot *applicationapiv1alpha1.Snapshot) bool {
statusCondition := meta.FindStatusCondition(snapshot.Status.Conditions, AppStudioTestSucceededCondition)
if statusCondition == nil {
statusCondition = meta.FindStatusCondition(snapshot.Status.Conditions, LegacyTestSucceededCondition)
return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown
}
return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown // ← always true: nil check is impossible here
}The if statusCondition == nil block returns early, so the final return on line 709 is only reachable when statusCondition from FindStatusCondition(AppStudioTestSucceededCondition) is already non-nil. The statusCondition != nil guard there can never be false — the analyser can prove it statically.
Both branches also have identical return expressions, which is the clearest signal that the control flow can be simplified.
Expected behaviour (unchanged)
- Neither condition set →
false
LegacyTestSucceededCondition set with non-Unknown status → trueAppStudioTestSucceededCondition set with non-Unknown status → true
Fix
Collapse the duplicated return into a single one after the fallback assignment:
func HaveAppStudioTestsFinished(snapshot *applicationapiv1alpha1.Snapshot) bool {
statusCondition := meta.FindStatusCondition(snapshot.Status.Conditions, AppStudioTestSucceededCondition)
if statusCondition == nil {
statusCondition = meta.FindStatusCondition(snapshot.Status.Conditions, LegacyTestSucceededCondition)
}
return statusCondition != nil && statusCondition.Status != metav1.ConditionUnknown
}Additional context
The existing tests in gitops/snapshot_test.go exercise HaveAppStudioTestsFinished when no condition is set and when AppStudioTestSucceededCondition is set, but there is no test for the case where only LegacyTestSucceededCondition is set. A test for that path should be added alongside the fix (compare: HaveAppStudioTestsSucceeded already has a legacy-condition test at line 458).
HaveAppStudioTestsSucceeded (the function right below this one) does not have this problem — it correctly checks the legacy condition without duplicating the return.
What the linter reports
Go's
nilnessstatic analyser flags a tautological nil check inHaveAppStudioTestsFinished()ingitops/snapshot.go:Current code
The
if statusCondition == nilblock returns early, so the finalreturnon line 709 is only reachable whenstatusConditionfromFindStatusCondition(AppStudioTestSucceededCondition)is already non-nil. ThestatusCondition != nilguard there can never be false — the analyser can prove it statically.Both branches also have identical return expressions, which is the clearest signal that the control flow can be simplified.
Expected behaviour (unchanged)
falseLegacyTestSucceededConditionset with non-Unknown status →trueAppStudioTestSucceededConditionset with non-Unknown status →trueFix
Collapse the duplicated return into a single one after the fallback assignment:
Additional context
The existing tests in
gitops/snapshot_test.goexerciseHaveAppStudioTestsFinishedwhen no condition is set and whenAppStudioTestSucceededConditionis set, but there is no test for the case where onlyLegacyTestSucceededConditionis set. A test for that path should be added alongside the fix (compare:HaveAppStudioTestsSucceededalready has a legacy-condition test at line 458).HaveAppStudioTestsSucceeded(the function right below this one) does not have this problem — it correctly checks the legacy condition without duplicating the return.