Skip to content

Commit

Permalink
all: make vet happy
Browse files Browse the repository at this point in the history
	$ go vet ./...
	# cuelang.org/go/cue/errors
	cue/errors/errors.go:326:15: method Is(err error, target error) bool should have signature Is(error) bool
	cue/errors/errors.go:335:15: method As(err error, target interface{}) bool should have signature As(any) bool
	# cuelang.org/go/cue/ast/astutil
	cue/ast/astutil/resolve.go:207:6: unreachable code

The unreachable code warning is an easy one: just like in other bits of
disabled code, comment it out.

The Is/As signature warning is slightly more worrying.
Presumably due to a mistake, cue/errors.list never actually satisfied
the interfaces used by Go's errors.As and errors.Is.
The added parameter was never used, reinforcing that theory.

Adjust the two methods. We don't strictly need a regression test, given
that `go vet` will now spot that mistake in CI.

And, as per the above, add `go vet` to CI.

Signed-off-by: Daniel Martí <[email protected]>
Change-Id: Ie6f65a89fb41f232beede0ab732b14f0a0b831a1
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/537656
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Roger Peppe <[email protected]>
  • Loading branch information
mvdan committed May 5, 2022
1 parent 13918dd commit a58ac53
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ jobs:
run: go generate ./...
- name: Test
run: go test ./...
- if: matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04'
name: Check
run: go vet ./...
- if: ${{ matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04' }}
name: Test with -race
run: go test -race ./...
Expand Down
16 changes: 15 additions & 1 deletion cmd/cue/cmd/testdata/script/cmd_github.txt
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ jobs:
run: go generate ./...
- name: Test
run: go test ./...
- if: matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04'
name: Check
run: go vet ./...
- if: ${{ matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04' }}
name: Test with -race
run: go test -race ./...
Expand Down Expand Up @@ -1140,7 +1143,7 @@ test: _#bashWorkflow & {
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#earlyChecks, _#cacheGoModules, _#step & {
if: "${{ \(_#isMaster) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
}, _#goGenerate, _#goTest, _#goTestRace & {
}, _#goGenerate, _#goTest, _#goCheck, _#goTestRace & {
if: "${{ matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)' }}"
}, _#checkGitClean, _#pullThroughProxy, _#failCLBuild]
}
Expand Down Expand Up @@ -1407,6 +1410,17 @@ _#goTest: _#step & {
name: "Test"
run: "go test ./..."
}
_#goCheck: _#step & {
// These checks can vary between platforms, as different code can be built
// based on GOOS and GOARCH build tags.
// However, CUE does not have any such build tags yet, and we don't use
// dependencies that vary wildly between platforms.
// For now, to save CI resources, just run the checks on one matrix job.
// TODO: consider adding more checks as per https://github.com/golang/go/issues/42119.
if: "matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)'"
name: "Check"
run: "go vet ./..."
}
_#goTestRace: _#step & {
name: "Test with -race"
run: "go test -race ./..."
Expand Down
6 changes: 3 additions & 3 deletions cue/ast/astutil/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,9 @@ func (s *scope) insert(name string, n, link ast.Node) {
if _, ok := existing.node.(*ast.ImportSpec); ok {
return
// TODO:
s.errFn(n.Pos(), "conflicting declaration %s\n"+
"\tprevious declaration at %s",
name, existing.node.Pos())
// s.errFn(n.Pos(), "conflicting declaration %s\n"+
// "\tprevious declaration at %s",
// name, existing.node.Pos())
} else {
s.errFn(n.Pos(), "alias %q redeclared in same scope", name)
}
Expand Down
4 changes: 2 additions & 2 deletions cue/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func appendToList(a list, err Error) list {
// The zero value for an list is an empty list ready to use.
type list []Error

func (p list) Is(err, target error) bool {
func (p list) Is(target error) bool {
for _, e := range p {
if errors.Is(e, target) {
return true
Expand All @@ -332,7 +332,7 @@ func (p list) Is(err, target error) bool {
return false
}

func (p list) As(err error, target interface{}) bool {
func (p list) As(target interface{}) bool {
for _, e := range p {
if errors.As(e, target) {
return true
Expand Down
46 changes: 39 additions & 7 deletions cue/testdata/eval/github.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test: _#bashWorkflow & {
steps: [_#writeNetrcFile, _#installGo, _#checkoutCode, _#earlyChecks, _#cacheGoModules, _#step & {
if: "${{ \(_#isMaster) }}"
run: "echo CUE_LONG=true >> $GITHUB_ENV"
}, _#goGenerate, _#goTest, _#goTestRace & {
}, _#goGenerate, _#goTest, _#goCheck, _#goTestRace & {
if: "${{ matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)' }}"
}, _#checkGitClean, _#pullThroughProxy, _#failCLBuild]
}
Expand Down Expand Up @@ -325,6 +325,17 @@ _#goTest: _#step & {
name: "Test"
run: "go test ./..."
}
_#goCheck: _#step & {
// These checks can vary between platforms, as different code can be built
// based on GOOS and GOARCH build tags.
// However, CUE does not have any such build tags yet, and we don't use
// dependencies that vary wildly between platforms.
// For now, to save CI resources, just run the checks on one matrix job.
// TODO: consider adding more checks as per https://github.com/golang/go/issues/42119.
if: "matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)'"
name: "Check"
run: "go vet ./..."
}
_#goTestRace: _#step & {
name: "Test with -race"
run: "go test -race ./..."
Expand Down Expand Up @@ -1150,20 +1161,25 @@ import "strings"
run: (string){ "go test ./..." }
}
8: (#struct){
if: (string){ "matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04'" }
name: (string){ "Check" }
run: (string){ "go vet ./..." }
}
9: (#struct){
name: (string){ "Test with -race" }
run: (string){ "go test -race ./..." }
if: (string){ "${{ matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04' }}" }
}
9: (#struct){
10: (#struct){
name: (string){ "Check that git is clean post generate and tests" }
run: (string){ "test -z \"$(git status --porcelain)\" || (git status; git diff; false)" }
}
10: (#struct){
11: (#struct){
name: (string){ "Pull this commit through the proxy on master" }
run: (string){ "v=$(git rev-parse HEAD)\ncd $(mktemp -d)\ngo mod init mod.com\nGOPROXY=https://proxy.golang.org go get -d cuelang.org/go/cmd/cue@$v" }
if: (string){ "${{ github.ref == 'refs/heads/master' }}" }
}
11: (#struct){
12: (#struct){
if: (string){ "${{ startsWith(github.ref, 'refs/heads/ci/') && failure() }}" }
name: (string){ "Post any failures for this matrix entry" }
run: (string){ "curl -f -s -n -H \"Content-Type: application/json\" --request POST --data \"{\\\"tag\\\":\\\"trybot\\\",\\\"message\\\":\\\"Build failed for ${{ runner.os }}-${{ matrix.go-version }}; see ${{ github.event.repository.html_url }}/actions/runs/${{ github.run_id }} for more details\\\",\\\"labels\\\":{\\\"TryBot-Result\\\":-1}}\" https://review.gerrithub.io/a/changes/$(basename $(dirname $GITHUB_REF))/revisions/$(basename $GITHUB_REF)/review" }
Expand Down Expand Up @@ -1667,20 +1683,25 @@ import "strings"
run: (string){ "go test ./..." }
}
8: (#struct){
if: (string){ "matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04'" }
name: (string){ "Check" }
run: (string){ "go vet ./..." }
}
9: (#struct){
name: (string){ "Test with -race" }
run: (string){ "go test -race ./..." }
if: (string){ "${{ matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04' }}" }
}
9: (#struct){
10: (#struct){
name: (string){ "Check that git is clean post generate and tests" }
run: (string){ "test -z \"$(git status --porcelain)\" || (git status; git diff; false)" }
}
10: (#struct){
11: (#struct){
name: (string){ "Pull this commit through the proxy on master" }
run: (string){ "v=$(git rev-parse HEAD)\ncd $(mktemp -d)\ngo mod init mod.com\nGOPROXY=https://proxy.golang.org go get -d cuelang.org/go/cmd/cue@$v" }
if: (string){ "${{ github.ref == 'refs/heads/master' }}" }
}
11: (#struct){
12: (#struct){
if: (string){ "${{ startsWith(github.ref, 'refs/heads/ci/') && failure() }}" }
name: (string){ "Post any failures for this matrix entry" }
run: (string){ "curl -f -s -n -H \"Content-Type: application/json\" --request POST --data \"{\\\"tag\\\":\\\"trybot\\\",\\\"message\\\":\\\"Build failed for ${{ runner.os }}-${{ matrix.go-version }}; see ${{ github.event.repository.html_url }}/actions/runs/${{ github.run_id }} for more details\\\",\\\"labels\\\":{\\\"TryBot-Result\\\":-1}}\" https://review.gerrithub.io/a/changes/$(basename $(dirname $GITHUB_REF))/revisions/$(basename $GITHUB_REF)/review" }
Expand Down Expand Up @@ -2173,6 +2194,11 @@ import "strings"
name: (string){ "Test" }
run: (string){ "go test ./..." }
}
_#goCheck(:ci): (#struct){
if: (string){ "matrix.go-version == '1.18.x' && matrix.os == 'ubuntu-20.04'" }
name: (string){ "Check" }
run: (string){ "go vet ./..." }
}
_#goTestRace(:ci): (#struct){
name: (string){ "Test with -race" }
run: (string){ "go test -race ./..." }
Expand Down Expand Up @@ -2259,6 +2285,7 @@ import "strings"
}),
〈4;_#goGenerate〉,
〈4;_#goTest〉,
〈4;_#goCheck〉,
(〈4;_#goTestRace〉 & {
if: "${{ matrix.go-version == '\(〈5;_#latestStableGo〉)' && matrix.os == '\(〈5;_#linuxMachine〉)' }}"
}),
Expand Down Expand Up @@ -2550,6 +2577,11 @@ import "strings"
name: "Test"
run: "go test ./..."
})
_#goCheck: (〈0;_#step〉 & {
if: "matrix.go-version == '\(〈1;_#latestStableGo〉)' && matrix.os == '\(〈1;_#linuxMachine〉)'"
name: "Check"
run: "go vet ./..."
})
_#goTestRace: (〈0;_#step〉 & {
name: "Test with -race"
run: "go test -race ./..."
Expand Down
13 changes: 13 additions & 0 deletions internal/ci/workflows.cue
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ test: _#bashWorkflow & {
},
_#goGenerate,
_#goTest,
_#goCheck,
_#goTestRace & {
if: "${{ matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)' }}"
},
Expand Down Expand Up @@ -412,6 +413,18 @@ _#goTest: _#step & {
run: "go test ./..."
}

_#goCheck: _#step & {
// These checks can vary between platforms, as different code can be built
// based on GOOS and GOARCH build tags.
// However, CUE does not have any such build tags yet, and we don't use
// dependencies that vary wildly between platforms.
// For now, to save CI resources, just run the checks on one matrix job.
// TODO: consider adding more checks as per https://github.com/golang/go/issues/42119.
if: "matrix.go-version == '\(_#latestStableGo)' && matrix.os == '\(_#linuxMachine)'"
name: "Check"
run: "go vet ./..."
}

_#goTestRace: _#step & {
name: "Test with -race"
run: "go test -race ./..."
Expand Down

0 comments on commit a58ac53

Please sign in to comment.