Skip to content

Commit

Permalink
Remove ErrorOnNilableMap handling (#243)
Browse files Browse the repository at this point in the history
Reading from a nil map does not really panic and is actually used quite
frequently in Go. When NilAway was initially developed we added a
feature that errs when a nilable map is accessed, and guard it under an
internal feature flag for us to see more data points. Now that NilAway
is more mature and we have seen more real world examples, this PR
removes such handling and the stale feature flag.
  • Loading branch information
yuxincs committed May 14, 2024
1 parent a25d53b commit fe712a8
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 17 deletions.
12 changes: 0 additions & 12 deletions assertion/function/assertiontree/root_assertion_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -526,18 +526,6 @@ func (r *RootAssertionNode) consumeIndexExpr(expr ast.Expr) {
Guards: util.NoGuards(),
})
}

// reads of nilable maps should not necessarily produce errors - the flag config.ErrorOnNilableMapRead
// encodes this optionality and is currently set to false
if config.ErrorOnNilableMapRead && util.TypeIsDeeplyMap(t) {
r.AddConsumption(&annotation.ConsumeTrigger{
Annotation: &annotation.MapAccess{ConsumeTriggerTautology: &annotation.ConsumeTriggerTautology{}},
Expr: expr,
Guards: util.NoGuards(),
})
}
// there are some weird types that can show up here (*internal/abi.IntArgRegBitmap for example)
// so don't error out if we don't recognize the type just no-op
}

// AddComputation takes the knowledge that the expression expr has to be computed to generate any necessary assertions to
Expand Down
5 changes: 0 additions & 5 deletions config/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ package config
// to lower values, making it a good compromise for precise results.
const StableRoundLimit = 5

// ErrorOnNilableMapRead configures whether reading from nil maps should be considered an error.
// Since Go does not panic on this, right now we do not interpret it as one, but this could be
// considered undesirable behavior and worth catching in the future.
const ErrorOnNilableMapRead = false

// NilAwayNoInferString is the string that may be inserted into the docstring for a package to prevent
// NilAway from inferring the annotations for that package - this is useful for unit tests
const NilAwayNoInferString = "<nilaway no inference>"
Expand Down

0 comments on commit fe712a8

Please sign in to comment.