Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes go.18+ compatibility #5

Merged
merged 1 commit into from
Jan 2, 2023

Conversation

TripleDogDare
Copy link
Collaborator

go1.18 changes subtle things about the type system. Most likely do to the addition of generics. The serum analyzer is a casualty of that. This should fix the immediate problems although it's not entirely clear to me why or if there's another option for handling it.

Testing

go1.18 test ./... on master will result in errors for Cause functions that should be ignored.
Example:

    analysistest.go:446: 001/main.go:220:1: unexpected diagnostic: function "Cause" is exported, but does not declare any error codes

Run the tests with both go1.17 AND go1.18+ to see the difference on both master and this branch.
The master branch still has other errors that may be unimplemented features (use a flag/skip?) or may need to be fixed. This PR does not address that issue.
Here's the expected output on master for go1.17

$ go1.17 test ./...
--- FAIL: TestVerifyAnalyzer (1.19s)
    analysistest.go:446: 001/main.go:153:1: unexpected diagnostic: function "DereferenceAssignment" has a mismatch of declared and actual error codes: unused codes: [other-error]
    analysistest.go:446: 001/main.go:164:1: unexpected diagnostic: function "DereferenceAssignment2" has a mismatch of declared and actual error codes: unused codes: [other-error]
    analysistest.go:446: multifile/file1.go:18:9: unexpected diagnostic: function "StringError" in dot-imported package does not declare error codes
    analysistest.go:446: multifile/file1.go:17:1: unexpected diagnostic: function "DoString" has a mismatch of declared and actual error codes: unused codes: [string-error]
    analysistest.go:446: multipackage/multipackage.go:139:9: unexpected diagnostic: function "StringError" in package "inner1" does not declare error codes
    analysistest.go:446: multipackage/multipackage.go:138:1: unexpected diagnostic: function "DoString" has a mismatch of declared and actual error codes: unused codes: [string-error]
FAIL
FAIL	github.com/serum-errors/go-serum-analyzer/analysis	1.203s
ok  	github.com/serum-errors/go-serum-analyzer/analysis/scc	(cached)
?   	github.com/serum-errors/go-serum-analyzer/cmd/go-serum-analyzer	[no test files]
FAIL

@warpfork
Copy link
Collaborator

warpfork commented Jan 2, 2023

Interesting. I confirm that this patch fixes problems relating to failure to recognize the "error with cause" type. And I also do not entirely understand why. 😅

It's changing the pattern of the "error with cause" interface to expect the "Cause" method to match the universally pre-declared error type instead of merely matching anything interface { Error() string } (which is functionally equivalent).

I don't understand why the latter worked in Go before 1.17 and wouldn't in 1.18.

That said, I don't think this change removes any meaningful functionality or flexibility, and it evidently solves problems, and therefore it seems reasonable to merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants