Turn on the golangci-lint exhaustive linter. This is the first step
towards catching more missing cases during development rather than
in tests, or in production.
This might be best reviewed commit-by-commit, as the first commit turns
on the linter with the `default-signifies-exhaustive: true` option set,
which requires a lot less changes in the current codebase.
I think it's probably worth doing the second commit as well, as that
will get us the real benefits, even though we end up with a little bit
more churn. However it means all the `switch` statements are covered,
which isn't the case after the first commit, since we do have a lot of
`default` statements that just call `assert.Fail`.
Fixes#14601
## Checklist
- [x] I have run `make tidy` to update any new dependencies
- [x] I have run `make lint` to verify my code passes the lint check
- [x] I have formatted my code using `gofumpt`
<!--- Please provide details if the checkbox below is to be left
unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my
feature works
<!---
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the
`changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the
Pulumi Cloud,
then the service should honor older versions of the CLI where this
change would not exist.
You must then bump the API version in
/pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi
Cloud API version
<!-- @Pulumi employees: If yes, you must submit corresponding changes in
the service repo. -->
This value was never actually used or correctly implemented, it was just
being used as a sentinal value for `Set/GetGlobalColorization`. We can
just use a pointer instead.
diag.DefaultSink is not safe for concurrent use.
Whether we use it with os.File or with a bytes.Buffer,
both targets do not synchronize their writes,
so this leaves room for interleaving of messages.
Making diag.DefaultSink thread-safe is generally desirable,
but is also a requirement for #12438
so that it can run upgrades concurrently
and print warnings as tasks fail.
This makes the result of DefaultSink thread-safe
by adding a mutex around the targeted stdout and stderr.
Note:
As a special case, when stdout and stderr are the same
we want to use the same mutex for them.
This matches the behavior of [os/exec.Cmd][1]
// If Stdout and Stderr are the same writer, and have a type that can
// be compared with ==, at most one goroutine at a time will call Write.
Stdout io.Writer
Stderr io.Writer
[1]: https://pkg.go.dev/os/exec#Cmd
A writer implementation can be anything,
and is not guaranteed to be comparable with `==`.
When that happens, the `==` will panic.
To prevent regressions, we have to handle that case
and treat uncomparable writers as different.
This is also similar to how os/exec does it:
https://cs.opensource.google/go/go/+/refs/tags/go1.20.2:src/os/exec/exec.go;l=475
Per team discussion, switching to gofumpt.
[gofumpt][1] is an alternative, stricter alternative to gofmt.
It addresses other stylistic concerns that gofmt doesn't yet cover.
[1]: https://github.com/mvdan/gofumpt
See the full list of [Added rules][2], but it includes:
- Dropping empty lines around function bodies
- Dropping unnecessary variable grouping when there's only one variable
- Ensuring an empty line between multi-line functions
- simplification (`-s` in gofmt) is always enabled
- Ensuring multi-line function signatures end with
`) {` on a separate line.
[2]: https://github.com/mvdan/gofumpt#Added-rules
gofumpt is stricter, but there's no lock-in.
All gofumpt output is valid gofmt output,
so if we decide we don't like it, it's easy to switch back
without any code changes.
gofumpt support is built into the tooling we use for development
so this won't change development workflows.
- golangci-lint includes a gofumpt check (enabled in this PR)
- gopls, the LSP for Go, includes a gofumpt option
(see [installation instrutions][3])
[3]: https://github.com/mvdan/gofumpt#installation
This change was generated by running:
```bash
gofumpt -w $(rg --files -g '*.go' | rg -v testdata | rg -v compilation_error)
```
The following files were manually tweaked afterwards:
- pkg/cmd/pulumi/stack_change_secrets_provider.go:
one of the lines overflowed and had comments in an inconvenient place
- pkg/cmd/pulumi/destroy.go:
`var x T = y` where `T` wasn't necessary
- pkg/cmd/pulumi/policy_new.go:
long line because of error message
- pkg/backend/snapshot_test.go:
long line trying to assign three variables in the same assignment
I have included mention of gofumpt in the CONTRIBUTING.md.
12267: chore(all): strings.Replace(..., -1) => strings.Replace(...) r=abhinav a=abhinav
Replaces all instances of `strings.Replace(s, old, new, -1)`
with the equivalent `strings.ReplaceAll(s, old, new)`.
This function has been available since Go 1.12.
12268: chore: WriteString(Sprintf(..)) => Fprintf(..) r=abhinav a=abhinav
Replace `buffer.WriteString(fmt.Sprintf(..))` calls,
where buffer is one of `bytes.Buffer`, `strings.Builder`, or `bufio.Writer`,
with equivalent `fmt.Fprintf` calls -- all those types are io.Writers.
12337: Freeze v3.56.0 r=dixler a=dixler
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
Co-authored-by: Kyle Dixler <kyle@pulumi.com>
Replaces all instances of `strings.Replace(s, old, new, -1)`
with the equivalent `strings.ReplaceAll(s, old, new)`.
This function has been available since Go 1.12.
Replace `buffer.WriteString(fmt.Sprintf(..))` calls,
where buffer is one of `bytes.Buffer`, `strings.Builder`, or `bufio.Writer`,
with equivalent `fmt.Fprintf` calls -- all those types are io.Writers.
Migrates all uses of contract.{Assert, AssertNoError, Require} in sdk/
to the `*f` variants that are required to provide more error context.
Step towards deprecating non-f variants entirely.
For context, `contract.Require` is similar to `contract.Assert`,
except it has a required parameter name as an argument:
func Require(cond bool, param string)
func Requiref(cond bool, param string, msg string, args ...any)
It includes the parameter name in the error message by default,
so the `msg` and `args` should only describe the constraint
without naming the parameter.
Refs #12132
The number of Unicode code points in a string is not the same as the
number of user-visible characters (graphemes). When measuring colorized
strings, we want the latter rather than the former. Notably, these
changes fix some issues where the interactive display cut off before the
right edge of the terminal.
We need to check that for example that if a resource X is created that
we don't allow another resource Y to be alias against X. Likewise if our
old state has X and we then create Y aliased against X we should not
then allow X to be created later in the deployment.
Fixes https://github.com/pulumi/pulumi/issues/11173
* about error string: error string should not be capitalized or end with punctuation mark
* apply suggestions from code review
* lint: fix format error and simplify the code
Co-authored-by: Fraser Waters <frassle@gmail.com>
Co-authored-by: Aaron Friel <mayreply@aaronfriel.com>
* Send smart aliases via gRPC to engine
* Add to SupportsFeature
* Restore old logic when the engine doesn't support smartAliases
* Add to deploytest ResourceOptions
* Add tests
* Add to CHANGELOG
* Fix test
* Rename proto fields
* Regenerate protobufs
* Fix up SDKs after field rename
* Rename deploytest aliases
* Rename internal fields
* Fix typo in c# code
* Fix typescript
* Rename feature to aliasSpecs
* Rename type to Spec
* Allow disabling default providers
This is done with an opt-in setting in `pulumi config`. For example, to
disable default providers for `aws`, use:
```sh
pulumi config set --path pulumi:disable-default-providers[0] aws
```
To add `kubernetes` to the disabled list, use
```sh
pulumi config set --path pulumi:disable-default-providers[1] kubernetes
```
To disable all default providers, `*` can be used.
---
Under the hood, whenever we handle a default provider request (with
`defaultProviders.handleRequest`), we make sure it isn't on the deny
list. If it is, we replace the requested reference with a special
`DenyDefaultProvider` reference. We check for this reference whenever we
are about to get a provider to do actual work. By intercepting denied
providers when references are created, we ensure that we never use a
denied provider.
* Update CHANGELOG_PENDING.md
* Fix lints
The lint errors appear to be unrelated to the original PR. Fixing them
unblocks the CI.
* Add engine tests
* Fix nits
* Clarify function
* Make `async:true` the default for `invoke` calls (#3750)
* Switch away from native grpc impl. (#3728)
* Remove usage of the 'deasync' library from @pulumi/pulumi. (#3752)
* Only retry as long as we get unavailable back. Anything else continues. (#3769)
* Handle all errors for now. (#3781)
* Do not assume --yes was present when using pulumi in non-interactive mode (#3793)
* Upgrade all paths for sdk and pkg to v2
* Backport C# invoke classes and other recent gen changes (#4288)
Adjust C# generation
* Replace IDeployment with a sealed class (#4318)
Replace IDeployment with a sealed class
* .NET: default to args subtype rather than Args.Empty (#4320)
* Adding system namespace for Dotnet code gen
This is required for using Obsolute attributes for deprecations
```
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'ObsoleteAttribute' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
Iam/InstanceProfile.cs(142,10): error CS0246: The type or namespace name 'Obsolete' could not be found (are you missing a using directive or an assembly reference?) [/Users/stack72/code/go/src/github.com/pulumi/pulumi-aws/sdk/dotnet/Pulumi.Aws.csproj]
```
* Fix the nullability of config type properties in C# codegen (#4379)