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

missing diagnostics for switch statements that switch on a value whose type is a type parameter #31

Closed
nishanths opened this issue Nov 16, 2021 · 3 comments
Assignees

Comments

@nishanths
Copy link
Owner

It is unclear to me at this point what effect the upcoming go1.18 type parameter change will have on this analyzer. We may need to define the analyzer's behavior when type parameters are involved. The analzyer implementation may also need to be updated to handle type parameters.

@ghostsquad
Copy link

now that 1.18 is out, does anyone know if this still works?

@nishanths
Copy link
Owner Author

nishanths commented Jun 3, 2022

It continues to work if the switch statement doesn't switch on a type-parameterized type.

However, if the switch statements switches on a type-parameterized type, exhaustive fails to detect missing cases. For example, for the switch statement in the function foo below, I think it should have reported missing B. In bar, I think it should have reported missing B and C.

package typeparam

type M int

const (
	A M = iota
	B
)

type N uint

const (
	C N = iota
	D
)

func foo[T M](v T) {
	switch v {
	case T(A):
	}
}

func bar[T M | N](v T) {
	switch v {
	case T(A):
	case T(D):
	}
}

One of the issues that should be addressed is that exhaustive doesn't check these switch statement because of an early-out return in switch.go.

exhaustive/switch.go

Lines 94 to 99 in c85349a

tagType, ok := t.Type.(*types.Named)
if !ok {
return true, resultTagNotNamed
}

(The type is *types.TypeParam, not *types.Named, for the example above.)

@nishanths nishanths changed the title prepare for go1.18 type parameters missing diagnostics for switch statements that switch on a type parameterized type Jun 3, 2022
@nishanths nishanths self-assigned this Nov 24, 2022
@nishanths
Copy link
Owner Author

nishanths commented Nov 24, 2022

Work-in-progress definition of the analyzer's behavior for type parameters.

Initial version

# Type parameters

A switch statement that switches on a value whose type is a type
parameter is checked for exhaustiveness iff each term of the type
constraint is an enum type. The following switch statement will be
checked, assuming M, N, and O are enum types. To satisfy exhaustiveness,
all enum members for each of M, N, and O must be listed in the switch
statement's cases.

	func bar[T M | I](v T) {
		switch v {
		}
	}

	type I interface {
		N | O
	}

Update 1

# Type parameters

A switch statement that switches on a value whose type is a type parameter is
checked for exhaustiveness iff each type element in the type constraint is
an enum type and shares the same underlying basic kind. For example, the
following switch statement will be checked, assuming M, N, and O are enum
types with the same underlying basic kind. To satisfy exhaustiveness, all enum
members for each of the types M, N, and O must be listed in the switch
statement's cases.

	func bar[T M | I](v T) {
		switch v {
		}
	}
	type I interface{ N | J }
	type J interface{ O }

@nishanths nishanths changed the title missing diagnostics for switch statements that switch on a type parameterized type missing diagnostics for switch statements that switch on a value whose type is a type parameter Nov 24, 2022
nishanths added a commit that referenced this issue Nov 26, 2022
Type parameters

This commit implements support for checking exhaustiveness when the
value's type is a type parameter. The defined behavior is (copied from
package documentation):

    A switch statement that switches on a value whose type is a type
    parameter is checked for exhaustiveness iff each type element in the
    type constraint is an enum type and shares the same underlying basic
    kind. For example, the following switch statement will be checked,
    assuming M, N, and O are enum types with the same underlying basic
    kind. To satisfy exhaustiveness, all enum members for each of the
    types M, N, and O must be listed in the switch statement's cases.

        func bar[T M | I](v T) {
            switch v {
            }
        }
        type I interface{ N | J }
        type J interface{ O }

Exhaustive supports go1.14 and later, but type parameters are only
supported from go1.18 or later. Symbols in e.g. package go/types related
to type parameters are not available in go version earlier than go1.18.
So source files, when necessary, are split into *_pre_go118.go and
*_go118.go portions.

The majority of the implementation is in common_go118.go.

Spurious diagnostic for type conversion (issue 42)

This commit also fixes spurious diagnostics that were produced when the
case clause expressions involved type conversions. See the relevant func
stripTypeConversions, and tests in testdata/general/x/typeconv.go.

Diagnostic format changes

The output diagnostics now always include the package name for the enum
type and the enum members. (Previously the package name was omitted if
the symbols were in the same package as the offending source code.) This
change was motivated by consistency: With type parameters, a value's
type in a switch statement tag can be composed of multiple enum types,
possibly from different packages. In such scenarios, consistently using
the package name for all of them makes for more comprehensible
diagnostics.

Many testdata lines are touched due to the diagnostic format change.

Internal changes

* Flag parsing: Parsing of the -check flag is now handled by a new
  stringsFlag type that implements flag.Value.
* Shared code: Code that was shared (or should have been shared between)
  switch.go and maps.go now lives in common.go.
* The enumMembers fact type has an additional NameToPos field.
* The checklist type is updated to account for the fact that enum types
  can possibly be from multiple packages.
* The Makefile now includes a "cover" target (HTML test coverage report)
  and an "all" target (runs build, vet, test).
* The new "member" type represents the concept of an enum member in
  switch.go and map.go API, replacing the prev. largely string-based
  API.

GitHub workflows

The workflow now runs tests for go versions < 1.16.

Fixes #31, #42
nishanths added a commit that referenced this issue Nov 26, 2022
Type parameters

This commit implements support for checking exhaustiveness when the
value's type is a type parameter. The defined behavior is (copied from
package documentation):

    A switch statement that switches on a value whose type is a type
    parameter is checked for exhaustiveness iff each type element in the
    type constraint is an enum type and shares the same underlying basic
    kind. For example, the following switch statement will be checked,
    assuming M, N, and O are enum types with the same underlying basic
    kind. To satisfy exhaustiveness, all enum members for each of the
    types M, N, and O must be listed in the switch statement's cases.

        func bar[T M | I](v T) {
            switch v {
            }
        }
        type I interface{ N | J }
        type J interface{ O }

Exhaustive supports go1.14 and later, but type parameters are only
supported from go1.18 or later. Symbols in e.g. package go/types related
to type parameters are not available in go version earlier than go1.18.
So source files, when necessary, are split into *_pre_go118.go and
*_go118.go portions.

The majority of the implementation is in common_go118.go.

Spurious diagnostic for type conversion (issue 42)

This commit also fixes spurious diagnostics that were produced when the
case clause expressions involved type conversions. See the relevant func
stripTypeConversions, and tests in testdata/general/x/typeconv.go.

Diagnostic format changes

The output diagnostics now always include the package name for the enum
type and the enum members. (Previously the package name was omitted if
the symbols were in the same package as the offending source code.) This
change was motivated by consistency: With type parameters, a value's
type in a switch statement tag can be composed of multiple enum types,
possibly from different packages. In such scenarios, consistently using
the package name for all of them makes for more comprehensible
diagnostics.

Many testdata lines are touched due to the diagnostic format change.

Internal changes

* Flag parsing: Parsing of the -check flag is now handled by a new
  stringsFlag type that implements flag.Value.
* Shared code: Code that was shared (or should have been shared between)
  switch.go and maps.go now lives in common.go.
* The enumMembers fact type has an additional NameToPos field.
* The checklist type is updated to account for the fact that enum types
  can possibly be from multiple packages.
* The Makefile now includes a "cover" target (HTML test coverage report)
  and an "all" target (runs build, vet, test).
* The new "member" type represents the concept of an enum member in
  switch.go and map.go API, replacing the prev. largely string-based
  API.
* Add staticheck as a vet tool.

GitHub workflows

The workflow now runs tests for go versions < 1.16.

Fixes #31, #42
nishanths added a commit that referenced this issue Nov 26, 2022
Type parameters

This commit implements support for checking exhaustiveness when the
value's type is a type parameter. The defined behavior is (copied from
package documentation):

    A switch statement that switches on a value whose type is a type
    parameter is checked for exhaustiveness iff each type element in the
    type constraint is an enum type and shares the same underlying basic
    kind. For example, the following switch statement will be checked,
    assuming M, N, and O are enum types with the same underlying basic
    kind. To satisfy exhaustiveness, all enum members for each of the
    types M, N, and O must be listed in the switch statement's cases.

        func bar[T M | I](v T) {
            switch v {
            }
        }
        type I interface{ N | J }
        type J interface{ O }

Exhaustive supports go1.14 and later, but type parameters are only
supported from go1.18 or later. Symbols in e.g. package go/types related
to type parameters are not available in go version earlier than go1.18.
So source files, when necessary, are split into *_pre_go118.go and
*_go118.go portions.

The majority of the implementation is in common_go118.go.

Spurious diagnostic for type conversion (issue 42)

This commit also fixes spurious diagnostics that were produced when the
case clause expressions involved type conversions. See the relevant func
stripTypeConversions, and tests in testdata/general/x/typeconv.go.

Diagnostic format changes

The output diagnostics now always include the package name for the enum
type and the enum members. (Previously the package name was omitted if
the symbols were in the same package as the offending source code.) This
change was motivated by consistency: With type parameters, a value's
type in a switch statement tag can be composed of multiple enum types,
possibly from different packages. In such scenarios, consistently using
the package name for all of them makes for more comprehensible
diagnostics.

Many testdata lines are touched due to the diagnostic format change.

Internal changes

* Flag parsing: Parsing of the -check flag is now handled by a new
  stringsFlag type that implements flag.Value.
* Shared code: Code that was shared (or should have been shared between)
  switch.go and maps.go now lives in common.go.
* The enumMembers fact type has an additional NameToPos field.
* The checklist type is updated to account for the fact that enum types
  can possibly be from multiple packages.
* The Makefile now includes a "cover" target (HTML test coverage report)
  and an "all" target (runs build, vet, test).
* The new "member" type represents the concept of an enum member in
  switch.go and map.go API, replacing the prev. largely string-based
  API.
* Add staticheck as a vet tool.

GitHub workflows

The workflow now runs tests for go versions < 1.16.

Fixes #31, #42

This is a squashed commit; see pull request #54 for individual commits.
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

No branches or pull requests

2 participants