|
| 1 | +# Proposal: check references to standard library packages inconsistent with go.mod go version |
| 2 | + |
| 3 | +Author(s): Jay Conrod based on discussion with Daniel Martí, Paul Jolly, Roger |
| 4 | +Peppe, Bryan Mills, and others. |
| 5 | + |
| 6 | +Last updated: 2021-05-12 |
| 7 | + |
| 8 | +Discussion at https://golang.org/issue/46136. |
| 9 | + |
| 10 | +## Abstract |
| 11 | + |
| 12 | +With this proposal, `go vet` (and `go test`) would report an error if a package |
| 13 | +imports a standard library package or references an exported standard library |
| 14 | +definition that was introduced in a higher version of Go than the version |
| 15 | +declared in the containing module's `go.mod` file. |
| 16 | + |
| 17 | +This makes the meaning of the `go` directive clearer and more consistent. As |
| 18 | +part of this proposal, we'll clarify the reference documentation and make a |
| 19 | +recommendation to module authors about how the `go` directive should be set. |
| 20 | +Specifically, the `go` directive should indicate the minimum version of Go that |
| 21 | +a module supports. Authors should set their `go` version to the minimum version |
| 22 | +of Go they're willing to support. Clients may or may not see errors when using a |
| 23 | +lower version of Go, for example, when importing a module package that imports a |
| 24 | +new standard library package or uses a new language feature. |
| 25 | + |
| 26 | +## Background |
| 27 | + |
| 28 | +The `go` directive was introduced in Go 1.12, shortly after modules were |
| 29 | +introduced in Go 1.11. |
| 30 | + |
| 31 | +At the time, there were several proposed language changes that seemed like they |
| 32 | +might be backward incompatible (collectively, "Go 2"). To avoid an incompatible |
| 33 | +split (like Python 2 and 3), we needed a way to declare the language version |
| 34 | +used by a set of packages so that Go 1 and Go 2 packages could be mixed together |
| 35 | +in the same build, compiled with different syntax and semantics. |
| 36 | + |
| 37 | +We haven't yet made incompatible changes to the language, but we have made some |
| 38 | +small compatible changes (binary literals added in 1.13). If a developer using |
| 39 | +Go 1.12 or older attempts to build a package with a binary literal (or any other |
| 40 | +unknown syntax), and the module containing the package declares Go 1.13 or |
| 41 | +higher, the compiler reports an error explaining the problem. The developer also |
| 42 | +sees an error in their own package if their `go.mod` file declares `go 1.12` or |
| 43 | +lower. |
| 44 | + |
| 45 | +In addition to language changes, the `go` directive has been used to opt in to |
| 46 | +new, potentially incompatible module behavior. In Go 1.14, the `go` version was |
| 47 | +used to enable automatic vendoring. In 1.17, the `go` version will control lazy |
| 48 | +module loading. |
| 49 | + |
| 50 | +One major complication is that access to standard library packages and features |
| 51 | +has not been consistently limited. For example, a module author might use |
| 52 | +`errors.Is` (added in 1.13) or `io/fs` (added in 1.16) while believing their |
| 53 | +module is compatible with a lower version of Go. The author shouldn't be |
| 54 | +expected to know this history, but they can't easily determine the lowest |
| 55 | +version of Go their module is compatible with. |
| 56 | + |
| 57 | +This complication has made the meaning of the `go` directive very murky. |
| 58 | + |
| 59 | +## Proposal |
| 60 | + |
| 61 | +We propose adding a new `go vet` analysis to report errors in packages that |
| 62 | +reference standard library packages and definitions that aren't available |
| 63 | +in the version of Go declared in the containing module's `go.mod` file. The |
| 64 | +analysis will cover imports, references to exported top-level definitions |
| 65 | +(functions, constants, etc.), and references to other exported symbols (fields, |
| 66 | +methods). |
| 67 | + |
| 68 | +The analysis should evaluate build constraints in source files (`// +build` |
| 69 | +and `//go:build` comments) as if the `go` version in the containing module's |
| 70 | +`go.mod` were the actual version of Go used to compile the package. The |
| 71 | +analysis should not consider imports and references in files that would only |
| 72 | +be built for higher versions of Go. |
| 73 | + |
| 74 | +This analysis should have no false positives, so it may be enabled by default |
| 75 | +in `go test`. |
| 76 | + |
| 77 | +Note that both `go vet` and `go test` report findings for packages named on |
| 78 | +the command line, but not their dependencies. `go vet all` may be used to check |
| 79 | +packages in the main module and everything needed to build them. |
| 80 | + |
| 81 | +The analysis would not report findings for standard library packages. |
| 82 | + |
| 83 | +The analysis would not be enabled in GOPATH mode. |
| 84 | + |
| 85 | +For the purpose of this proposal, modules lacking a `go` directive (including |
| 86 | +projects without a `go.mod` file) are assumed to declare Go 1.16. |
| 87 | + |
| 88 | +## Rationale |
| 89 | + |
| 90 | +When writing this proposal, we also considered restrictions in the `go` command |
| 91 | +and in the compiler. |
| 92 | + |
| 93 | +The `go` command parses imports and applies build constraints, so it can report |
| 94 | +an error if a package in the standard library should not be imported. However, |
| 95 | +this may break currently non-compliant builds in a way that module authors |
| 96 | +can't easily fix: the error may be in one of their dependencies. We could |
| 97 | +disable errors in packages outside the main module, but we still can't easily |
| 98 | +re-evaluate build constraints for a lower release version of Go. The `go` |
| 99 | +command doesn't type check packages, so it can't easily detect references |
| 100 | +to new definitions in standard library packages. |
| 101 | + |
| 102 | +The compiler does perform type checking, but it does not evaluate build |
| 103 | +constraints. The `go` command provides the compiler with a list of files to |
| 104 | +compile, so the compiler doesn't even know about files excluded by build |
| 105 | +constraints. |
| 106 | + |
| 107 | +For these reasons, a vet analysis seems like a better, consistent way to |
| 108 | +find these problems. |
| 109 | + |
| 110 | +## Compatibility |
| 111 | + |
| 112 | +The analysis in this proposal may introduce new errors in `go vet` and `go test` |
| 113 | +for packages that reference parts of the standard library that aren't available |
| 114 | +in the declared `go` version. Module authors can fix these errors by increasing |
| 115 | +the `go` version, changing usage (for example, using a polyfill), or guarding |
| 116 | +usage with build constraints. |
| 117 | + |
| 118 | +Errors should only be reported in packages named on the command line. Developers |
| 119 | +should not see errors in packages outside their control unless they test with |
| 120 | +`go test all` or something similar. For those tests, authors may use `-vet=off` |
| 121 | +or a narrower set of analyses. |
| 122 | + |
| 123 | +We may want to add this analysis to `go vet` without immediately enabling it by |
| 124 | +default in `go test`. While it should be safe to enable in `go test` (no false |
| 125 | +positives), we'll need to verify this is actually the case, and we'll need |
| 126 | +to understand how common these errors will be. |
| 127 | + |
| 128 | +## Implementation |
| 129 | + |
| 130 | +This proposal is targeted for Go 1.18. Ideally, it should be implemented |
| 131 | +at the same time or before generics, since there will be a lot of language |
| 132 | +and library changes around that time. |
| 133 | + |
| 134 | +The Go distribution includes files in the `api/` directory that track when |
| 135 | +packages and definitions were added to the standard library. These are used to |
| 136 | +guard against unintended changes. They're also used in pkgsite documentation. |
| 137 | +These files are the source of truth for this proposal. `cmd/vet` will access |
| 138 | +these files from `GOROOT`. |
| 139 | + |
| 140 | +The analysis can determine the `go` version for each package by walking up |
| 141 | +the file tree and reading the `go.mod` file for the containing module. If the |
| 142 | +package is in the module cache, the analysis will use the `.mod` file for the |
| 143 | +module version. This file is generated by the `go` command if no `go.mod` |
| 144 | +file was present in the original tree. |
| 145 | + |
| 146 | +Each analysis receives a set of parsed and type checked files from `cmd/vet`. |
| 147 | +If the proposed analysis detects that one or more source files (including |
| 148 | +ignored files) contain build constraints with release tags (like `go1.18`), |
| 149 | +the analysis will parse and type check the package again, applying a corrected |
| 150 | +set of release tags. The analysis can then look for inappropriate imports |
| 151 | +and references. |
| 152 | + |
| 153 | +## Related issues |
| 154 | + |
| 155 | +* [#30639](https://golang.org/issue/30639) |
| 156 | +* https://twitter.com/mvdan_/status/1391772223158034434 |
| 157 | +* https://twitter.com/marcosnils/status/1372966993784152066 |
| 158 | +* https://twitter.com/empijei/status/1382269202380251137 |
0 commit comments