-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
pkg/codegen: Add types for typeTokenKind values #11857
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
string constants don't behave like iota. With, type something string const ( foo something = "foo" bar = "bar" ) Only `foo` will have the type `something`. `bar` will be an untyped string literal. The type is copied over only when there's no rhs, e.g. type something int const ( foo something = iota bar ) This adds types for constants that were intended to have the type `typeTokenKind`. Issue found by staticcheck: ``` codegen/testing/tstypes/tstypes.go:157:2: SA9004: only the first constant in this group has an explicit type (staticcheck) ``` Refs #11808
abhinav
added
the
impact/no-changelog-required
This issue doesn't require a CHANGELOG update
label
Jan 13, 2023
Changelog[uncommitted] (2023-01-13) |
justinvp
approved these changes
Jan 13, 2023
bors merge |
bors bot
added a commit
that referenced
this pull request
Jan 13, 2023
11857: pkg/codegen: Add types for typeTokenKind values r=abhinav a=abhinav string constants don't behave like iota. With, type something string const ( foo something = "foo" bar = "bar" ) Only `foo` will have the type `something`. `bar` will be an untyped string literal. The type is copied over only when there's no rhs, e.g. type something int const ( foo something = iota bar ) This adds types for constants that were intended to have the type `typeTokenKind`. Issue found by staticcheck: ``` codegen/testing/tstypes/tstypes.go:157:2: SA9004: only the first constant in this group has an explicit type (staticcheck) ``` Refs #11808 11858: codgen/go/test: Replace ineffective sort r=abhinav a=abhinav `sort.StringSlice` is a type wrapper, not a function. type StringSlice []string So `ordering = sort.StringSlice(ordering)` has no effect. This replaces that with use of `sort.Strings`, whic has the desired effect of sorting the slice in place. Also drops the unnecesary index tracking to append into the slice in favor of appends. Issue caught by staticcheck: ``` codegen/go/gen_test.go:220:4: SA4029: sort.StringSlice is a type, not a function, and sort.StringSlice(ordering) doesn't sort your values; consider using sort.Strings instead (staticcheck) ``` Refs #11808 11859: sdk/go/generate: Replace incorrect TrimRight with TrimSuffix r=abhinav a=abhinav Code in go/pulumi/generate that determines the destination file path based on the name of the template incorrectly used TrimRight instead of TrimSuffix. TrimRight removes all instances of *any* of the provided characters (referred to as the cutset) from the provided string instead of removing just a fixed suffix. So if we tried to generate say, a .tmpl file, it would have stripped that from the destination file path too. This switches to TrimSuffix, which is what was intended here, and adds a test to verify the desired behavior. Issue caught by staticcheck: ``` go/pulumi/generate/main.go:343:54: SA1024: cutset contains duplicate characters (staticcheck) ``` Refs #11808 11860: pkg: Drop unnecessary printfs r=abhinav a=abhinav A couple places in pkg/ use Printf without any arguments. These cases can use the input strings as-is instead. This changes switches them to the Print variant. Issues found by staticcheck: backend/httpstate/backend.go:1627:2: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) backend/httpstate/backend.go:1644:5: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) codegen/go/gen.go:2260:2: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) codegen/nodejs/gen.go:1088:3: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) codegen/python/gen.go:1861:2: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) codegen/python/gen.go:436:2: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) Refs #11808 11862: pkg: Fix unnecessary appends r=abhinav a=abhinav Removes unnecessary appends in pkg. There were two instances: - append into a slice that's never used again - append with nothing to append (`append([]{foo}) == []{foo}`) Issue found by staticcheck: ``` testing/integration/program.go:2190:15: SA4021: x = append(y) is equivalent to x = y (staticcheck) testing/integration/program.go:2192:15: SA4021: x = append(y) is equivalent to x = y (staticcheck) codegen/pcl/utilities.go:158:11: SA4010: this result of append is never used, except maybe in other appends (staticcheck) ``` Refs #11808 11863: Improve err message on an invalid plugin override r=iwahbe a=iwahbe Just adding context to an error message. Co-authored-by: Abhinav Gupta <abhinav@pulumi.com> Co-authored-by: Ian Wahbe <ian@wahbe.com>
Build failed (retrying...): |
abhinav
added a commit
that referenced
this pull request
Jan 13, 2023
Remove staticcheck from the list of disabled linters. It's enabled by default in golangci-lint. This also fixes minor remaining staticcheck issues that don't merit their own pull requests, or opts out of those that cannot be fixed yet. Notably, we're opting out of: - Resource.Name is deprecated (#9469) - github.com/golang/protobuf is deprecated (#11869) - strings.Title has been deprecated (#11870) Besides that, other issues addressed in this change are: ``` // all issues are in pkg codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck) codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck) resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck) resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck) codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck) ``` Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, 11867, #11868 Resolves #11808
abhinav
added a commit
that referenced
this pull request
Jan 13, 2023
Remove staticcheck from the list of disabled linters. It's enabled by default in golangci-lint. This also fixes minor remaining staticcheck issues that don't merit their own pull requests, or opts out of those that cannot be fixed yet. Notably, we're opting out of: - Resource.Name is deprecated (#9469) - github.com/golang/protobuf is deprecated (#11869) - strings.Title has been deprecated (#11870) Besides that, other issues addressed in this change are: ``` // all issues are in pkg codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck) codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck) resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck) resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck) codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck) ``` Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868 Resolves #11808
Build succeeded:
|
abhinav
added a commit
that referenced
this pull request
Jan 14, 2023
Remove staticcheck from the list of disabled linters. It's enabled by default in golangci-lint. This also fixes minor remaining staticcheck issues that don't merit their own pull requests, or opts out of those that cannot be fixed yet. Notably, we're opting out of: - Resource.Name is deprecated (#9469) - github.com/golang/protobuf is deprecated (#11869) - strings.Title has been deprecated (#11870) Besides that, other issues addressed in this change are: ``` // all issues are in pkg codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck) codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck) resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck) resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck) codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck) ``` Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868 Resolves #11808
abhinav
added a commit
that referenced
this pull request
Jan 15, 2023
Remove staticcheck from the list of disabled linters. It's enabled by default in golangci-lint. This also fixes minor remaining staticcheck issues that don't merit their own pull requests, or opts out of those that cannot be fixed yet. Notably, we're opting out of: - Resource.Name is deprecated (#9469) - github.com/golang/protobuf is deprecated (#11869) - strings.Title has been deprecated (#11870) Besides that, other issues addressed in this change are: ``` // all issues are in pkg codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck) codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck) resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck) resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck) codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck) ``` Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868 Resolves #11808
bors bot
added a commit
that referenced
this pull request
Jan 15, 2023
11871: golangci-lint: Enable staticcheck r=RobbieMcKinstry a=abhinav Remove staticcheck from the list of disabled linters. It's enabled by default in golangci-lint. This also fixes minor remaining staticcheck issues that don't merit their own pull requests, or opts out of those that cannot be fixed yet. Notably, we're opting out of: - Resource.Name is deprecated (#9469) - github.com/golang/protobuf is deprecated (#11869) - strings.Title has been deprecated (#11870) Besides that, other issues addressed in this change are: ``` // all issues are in pkg codegen/schema/docs_parser.go:103:4: SA4006: this value of `text` is never used (staticcheck) codegen/schema/loader.go:253:3: SA9003: empty branch (staticcheck) resource/deploy/step_executor.go:328:12: SA9003: empty branch (staticcheck) resource/deploy/step_generator.go:141:10: SA9003: empty branch (staticcheck) codegen/pcl/invoke.go:97:10: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_const.go:57:2: SA9003: empty branch (staticcheck) codegen/hcl2/model/type_enum.go:99:9: SA4001: &*x will be simplified to x. It will not copy x. (staticcheck) codegen/go/gen_test.go:399:19: SA4017: HasPrefix is a pure function but its return value is ignored (staticcheck) ``` Depends on #11857, #11858, #11859, #11860, #11862, #11865, #11866, #11867, #11868 Resolves #11808 --- **NOTE**: This PR's base branch is currently #11868. The base branch will be updated when that lands. Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
string constants don't behave like iota.
With,
Only
foo
will have the typesomething
.bar
will be an untyped string literal.The type is copied over only when there's no rhs, e.g.
This adds types for constants
that were intended to have the type
typeTokenKind
.Issue found by staticcheck:
Refs #11808