Skip to content

Commit

Permalink
fix conflicting aliases producing random results because of map usage
Browse files Browse the repository at this point in the history
  • Loading branch information
grosser committed Oct 14, 2024
1 parent 07c67e5 commit 4e4c712
Show file tree
Hide file tree
Showing 8 changed files with 1,277 additions and 38 deletions.
2 changes: 1 addition & 1 deletion analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
)

var config = &Config{
RequiredAlias: make(map[string]string),
RequiredAlias: make([][]string, 0),
}

var Analyzer = &analysis.Analyzer{
Expand Down
66 changes: 37 additions & 29 deletions analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import (

func makeAnalyzer() *analysis.Analyzer {
cnf := Config{
RequiredAlias: make(map[string]string),
RequiredAlias: make([][]string, 0),
}
return &analysis.Analyzer{
Name: "importas",
Expand All @@ -43,9 +43,9 @@ func TestIncorrectFlags(t *testing.T) {
}

func TestConcurrency(t *testing.T) {
aliases := stringMap{
"fmt": "fff",
"os": "stdos",
aliases := aliasList{
[]string{"fmt", "fff"},
[]string{"os", "stdos"},
}
testdata := analysistest.TestData()
dir := filepath.Join(testdata, "src", "b")
Expand All @@ -64,8 +64,8 @@ func TestConcurrency(t *testing.T) {
}
a := makeAnalyzer()
flg := a.Flags.Lookup("alias")
for k, v := range aliases {
err := flg.Value.Set(fmt.Sprintf("%s:%s", k, v))
for _, alias := range aliases {
err := flg.Value.Set(fmt.Sprintf("%s:%s", alias[0], alias[1]))
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -100,49 +100,49 @@ func TestAnalyzer(t *testing.T) {
testCases := []struct {
desc string
pkg string
aliases stringMap
aliases aliasList
disallowUnaliased bool
disallowExtraAliases bool
}{
{
desc: "Invalid imports",
pkg: "a",
aliases: stringMap{
"fmt": "fff",
"os": "stdos",
"io": "iio",
aliases: aliasList{
[]string{"fmt", "fff"},
[]string{"os", "stdos"},
[]string{"io", "iio"},
},
},
{
desc: "Valid imports",
pkg: "b",
aliases: stringMap{
"fmt": "fff",
"os": "stdos",
aliases: aliasList{
[]string{"fmt", "fff"},
[]string{"os", "stdos"},
},
},
{
desc: "external libs",
pkg: "c",
aliases: stringMap{
"knative.dev/serving/pkg/apis/autoscaling/v1alpha1": "autoscalingv1alpha1",
"knative.dev/serving/pkg/apis/serving/v1": "servingv1",
aliases: aliasList{
[]string{"knative.dev/serving/pkg/apis/autoscaling/v1alpha1", "autoscalingv1alpha1"},
[]string{"knative.dev/serving/pkg/apis/serving/v1", "servingv1"},
},
},
{
desc: "regexp",
pkg: "d",
aliases: stringMap{
"knative.dev/serving/pkg/apis/(\\w+)/(v[\\w\\d]+)": "$1$2",
aliases: aliasList{
[]string{"knative.dev/serving/pkg/apis/(\\w+)/(v[\\w\\d]+)", "$1$2"},
},
},
{
desc: "disallow unaliased mode",
pkg: "e",
aliases: stringMap{
"fmt": "fff",
"os": "stdos",
"io": "iio",
aliases: aliasList{
[]string{"fmt", "fff"},
[]string{"os", "stdos"},
[]string{"io", "iio"},
},
disallowUnaliased: true,
},
Expand All @@ -154,18 +154,26 @@ func TestAnalyzer(t *testing.T) {
{
desc: "regexp with non capturing groups",
pkg: "g",
aliases: stringMap{
"knative.dev/serving/pkg/(?:apis/)?(\\w+)(?:/v[\\w\\d]+)?": "k$1",
aliases: aliasList{
[]string{"knative.dev/serving/pkg/(?:apis/)?(\\w+)(?:/v[\\w\\d]+)?", "k$1"},
},
},
{
desc: "dot imports should be handled correctly",
pkg: "h",
aliases: stringMap{
"github.com/onsi/gomega": ".",
aliases: aliasList{
[]string{"github.com/onsi/gomega", "."},
},
disallowUnaliased: true,
},
{
desc: "conflicting aliases",
pkg: "i",
aliases: aliasList{
[]string{"knative.dev/serving/pkg/apis/serving/v1", "special"}, // this goes first
[]string{"knative.dev/serving/pkg/apis/(\\w+)/(v[\\w\\d]+)", "$1$2"},
},
},
}

for _, test := range testCases {
Expand All @@ -189,8 +197,8 @@ func TestAnalyzer(t *testing.T) {
}
a := makeAnalyzer()
flg := a.Flags.Lookup("alias")
for k, v := range test.aliases {
err := flg.Value.Set(fmt.Sprintf("%s:%s", k, v))
for _, alias := range test.aliases {
err := flg.Value.Set(fmt.Sprintf("%s:%s", alias[0], alias[1]))
if err != nil {
t.Fatal(err)
}
Expand Down
5 changes: 3 additions & 2 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

type Config struct {
RequiredAlias map[string]string
RequiredAlias aliasList
Rules []*Rule
DisallowUnaliased bool
DisallowExtraAliases bool
Expand All @@ -22,7 +22,8 @@ func (c *Config) CompileRegexp() error {
return nil
}
rules := make([]*Rule, 0, len(c.RequiredAlias))
for path, alias := range c.RequiredAlias {
for _, aliases := range c.RequiredAlias {
path, alias := aliases[0], aliases[1]
reg, err := regexp.Compile(fmt.Sprintf("^%s$", path))
if err != nil {
return err
Expand Down
12 changes: 6 additions & 6 deletions flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,23 @@ var errWrongAlias = errors.New("import flag must be of form path:alias")

func flags(config *Config) flag.FlagSet {
fs := flag.FlagSet{}
fs.Var(stringMap(config.RequiredAlias), "alias", "required import alias in form path:alias")
fs.Var(&config.RequiredAlias, "alias", "required import alias in form path:alias")
fs.BoolVar(&config.DisallowUnaliased, "no-unaliased", false, "do not allow unaliased imports of aliased packages")
fs.BoolVar(&config.DisallowExtraAliases, "no-extra-aliases", false, "do not allow non-required aliases")
return fs
}

type stringMap map[string]string
type aliasList [][]string

func (v stringMap) Set(val string) error {
func (v *aliasList) Set(val string) error {
lastColon := strings.LastIndex(val, ":")
if lastColon <= 1 {
return errWrongAlias
}
v[val[:lastColon]] = val[lastColon+1:]
*v = append(*v, []string{val[:lastColon], val[lastColon+1:]})
return nil
}

func (v stringMap) String() string {
return fmt.Sprintf("%v", (map[string]string)(v))
func (v *aliasList) String() string {
return fmt.Sprintf("%v", ([][]string)(*v))
}
5 changes: 5 additions & 0 deletions testdata/src/i/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module github.com/julz/importas/testdata/src/d

go 1.15

require knative.dev/serving v0.21.0
Loading

0 comments on commit 4e4c712

Please sign in to comment.