Skip to content

Commit

Permalink
Add initial handling for iterators (#288)
Browse files Browse the repository at this point in the history
NilAway is panicking on code that ranges over iterators. This PR adds
initial handling for such that we assume the results from the iterators
are always nonnil, without panicking.

See #287
  • Loading branch information
yuxincs authored Oct 10, 2024
1 parent f74befd commit ba14292
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 0 deletions.
26 changes: 26 additions & 0 deletions assertion/function/assertiontree/backprop.go
Original file line number Diff line number Diff line change
Expand Up @@ -410,8 +410,34 @@ func backpropAcrossRange(rootNode *RootAssertionNode, lhs []ast.Expr, rhs ast.Ex
}
}

// produceNonNil marks the ith lhs expression as nonnil due to limitations of NilAway.
produceNonNil := func(i int) {
if !util.IsEmptyExpr(lhs[i]) {
rootNode.AddProduction(&annotation.ProduceTrigger{
Annotation: &annotation.ProduceTriggerNever{},
Expr: lhs[i],
})
}
}

rhsType := types.Unalias(rootNode.Pass().TypesInfo.Types[rhs].Type)

// Go 1.23 introduced the `iter` package, which provides a way to iterate over sequences
// in a generic way. The `iter.Seq` and `iter.Seq2` types are used to represent sequences
// and are used in the `range` statement. We currently do not handle these types yet, so
// here we assume that they are deeply non-nil (by adding nonnil producers).
// TODO: handle that (#287).
if named, ok := rhsType.(*types.Named); ok && named.Obj() != nil && named.Obj().Pkg() != nil && named.Obj().Pkg().Path() == "iter" {
if named.Obj().Name() == "Seq" {
produceNonNil(0)
return nil
} else if named.Obj().Name() == "Seq2" {
produceNonNil(0)
produceNonNil(1)
return nil
}
}

// This block breaks down the cases for the `range` statement being analyzed,
// starting by switching on how many left-hand operands there are
switch len(lhs) {
Expand Down
52 changes: 52 additions & 0 deletions nilaway_go123_plus_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright (c) 2023 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

//go:build go1.23

// This file is meant for testing features in Go 1.23 and beyond.
// TODO: Migrate these test cases in the mainstream test files once NilAway starts to support Go 1.23 is a base version.

package nilaway

import (
"testing"

"golang.org/x/tools/go/analysis/analysistest"
)

func TestNilAway_Go123(t *testing.T) {
t.Parallel()

testdata := analysistest.TestData()

// For descriptions of the purpose of each of the following tests, consult their source files
// located in testdata/src/<package>.

tests := []struct {
name string
patterns []string
}{
{name: "LoopRangeGo123", patterns: []string{"go.uber.org/looprangego123"}},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
t.Logf("Running test for packages %s", tt.patterns)

analysistest.Run(t, testdata, Analyzer, tt.patterns...)
})
}
}
33 changes: 33 additions & 0 deletions testdata/src/go.uber.org/looprangego123/looprangego123.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

// This package aims to test nilability behavior for `range` in loops.

// <nilaway no inference>
package looprange

import (
"maps"
"slices"
)

func testIter() {
i := 42
for element := range slices.Values([]*int{&i, &i, nil}) {
print(*element) // FN: we do not really handle iterators for now, the elements from iterators are assumed to be nonnil.
}
for k, v := range maps.All(map[string]*int{"abc": &i, "def": nil}) {
print(k)
print(*v) // FN: we do not really handle iterators for now, the elements from iterators are assumed to be nonnil.
}
}

0 comments on commit ba14292

Please sign in to comment.