Strict mode (#4)

* Add new test case

* Fix wrong test description

* Implement strict mode

* Add strict flag

* Add testcase

* Add test case for strict mode

* Prepare config instances for each test case

* Add README about strict mode

* Apply suggestions from code review

Co-authored-by: Julian Friedman <julz.friedman@uk.ibm.com>

* Fix for review

* Rename -strict flag to -no-unaliased

Co-authored-by: Julian Friedman <julz.friedman@uk.ibm.com>

README.md

@@ -19,6 +19,18 @@ importas \
   ./...
 ~~~~
 
+### `-no-unaliased` option
+
+By default, importas allows non-aliased imports, even when the package is specified by `-alias` flag.
+With `-no-unaliased` option, importas does not allow this.
+
+~~~~
+importas -no-unaliased \
+  -alias knative.dev/serving/pkg/apis/autoscaling/v1alpha1:autoscalingv1alpha1 \
+  -alias knative.dev/serving/pkg/apis/serving/v1:servingv1 \
+  ./...
+~~~~
+
 ### Use regular expression
 
 You can specify the package path by regular expression, and alias by regular expression replacement syntax like following snippet.

analyzer.go

@@ -12,20 +12,20 @@ import (
 	"golang.org/x/tools/go/ast/inspector"
 )
 
+var config = &Config{
+	RequiredAlias: make(map[string]string),
+}
+
 var Analyzer = &analysis.Analyzer{
 	Name: "importas",
 	Doc:  "Enforces consistent import aliases",
 	Run:  run,
 
-	Flags: flags(),
+	Flags: flags(config),
 
 	Requires: []*analysis.Analyzer{inspect.Analyzer},
 }
 
-var config = &Config{
-	RequiredAlias: make(map[string]string),
-}
-
 func run(pass *analysis.Pass) (interface{}, error) {
 	return runWithConfig(config, pass)
 }
@@ -37,18 +37,22 @@ func runWithConfig(config *Config, pass *analysis.Pass) (interface{}, error) {
 
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	inspect.Preorder([]ast.Node{(*ast.ImportSpec)(nil)}, func(n ast.Node) {
-		visitImportSpecNode(n.(*ast.ImportSpec), pass)
+		visitImportSpecNode(config, n.(*ast.ImportSpec), pass)
 	})
 
 	return nil, nil
 }
 
-func visitImportSpecNode(node *ast.ImportSpec, pass *analysis.Pass) {
-	if node.Name == nil {
-		return // not aliased at all, ignore. (Maybe add strict mode for this?).
+func visitImportSpecNode(config *Config, node *ast.ImportSpec, pass *analysis.Pass) {
+	if !config.DisallowUnaliased && node.Name == nil {
+		return
+	}
+
+	alias := ""
+	if node.Name != nil {
+		alias = node.Name.String()
 	}
 
-	alias := node.Name.String()
 	if alias == "." {
 		return // Dot aliases are generally used in tests, so ignore.
 	}
@@ -63,10 +67,15 @@ func visitImportSpecNode(node *ast.ImportSpec, pass *analysis.Pass) {
 	}
 
 	if required, exists := config.AliasFor(path); exists && required != alias {
+		message := fmt.Sprintf("import %q imported as %q but must be %q according to config", path, alias, required)
+		if alias == "" {
+			message = fmt.Sprintf("import %q imported without alias but must be with alias %q according to config", path, required)
+		}
+
 		pass.Report(analysis.Diagnostic{
 			Pos:     node.Pos(),
 			End:     node.End(),
-			Message: fmt.Sprintf("import %q imported as %q but must be %q according to config", path, alias, required),
+			Message: message,
 			SuggestedFixes: []analysis.SuggestedFix{{
 				Message:   "Use correct alias",
 				TextEdits: findEdits(node, pass.TypesInfo.Uses, path, alias, required),
@@ -96,13 +105,11 @@ func findEdits(node ast.Node, uses map[*ast.Ident]types.Object, importPath, orig
 			continue
 		}
 
-		if original == pkgName.Name() {
-			result = append(result, analysis.TextEdit{
-				Pos:     use.Pos(),
-				End:     use.End(),
-				NewText: []byte(required),
-			})
-		}
+		result = append(result, analysis.TextEdit{
+			Pos:     use.Pos(),
+			End:     use.End(),
+			NewText: []byte(required),
+		})
 	}
 
 	return result

analyzer_test.go

@@ -5,29 +5,34 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strconv"
 	"testing"
 
+	"golang.org/x/tools/go/analysis"
 	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/inspect"
 )
 
 func TestAnalyzer(t *testing.T) {
 	testdata := analysistest.TestData()
 
 	testCases := []struct {
-		desc    string
-		pkg     string
-		aliases stringMap
+		desc              string
+		pkg               string
+		aliases           stringMap
+		disallowUnaliased bool
 	}{
 		{
-			desc: "Valid imports",
+			desc: "Invalid imports",
 			pkg:  "a",
 			aliases: stringMap{
 				"fmt": "fff",
 				"os":  "stdos",
+				"io":  "iio",
 			},
 		},
 		{
-			desc: "Invalid imports",
+			desc: "Valid imports",
 			pkg:  "b",
 			aliases: stringMap{
 				"fmt": "fff",
@@ -49,6 +54,16 @@ func TestAnalyzer(t *testing.T) {
 				"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",
+			},
+			disallowUnaliased: true,
+		},
 	}
 
 	for _, test := range testCases {
@@ -71,7 +86,17 @@ func TestAnalyzer(t *testing.T) {
 				}
 			}
 
-			a := Analyzer
+			cnf := Config{
+				RequiredAlias: make(map[string]string),
+			}
+			flgs := flags(&cnf)
+			a := &analysis.Analyzer{
+				Flags: flgs,
+				Run: func(pass *analysis.Pass) (interface{}, error) {
+					return runWithConfig(&cnf, pass)
+				},
+				Requires: []*analysis.Analyzer{inspect.Analyzer},
+			}
 
 			flg := a.Flags.Lookup("alias")
 			for k, v := range test.aliases {
@@ -81,7 +106,12 @@ func TestAnalyzer(t *testing.T) {
 				}
 			}
 
-			analysistest.RunWithSuggestedFixes(t, testdata, Analyzer, test.pkg)
+			noUnaliasedFlg := a.Flags.Lookup("no-unaliased")
+			if err := noUnaliasedFlg.Value.Set(strconv.FormatBool(test.disallowUnaliased)); err != nil {
+				t.Fatal(err)
+			}
+
+			analysistest.RunWithSuggestedFixes(t, testdata, a, test.pkg)
 		})
 	}
 }

config.go

@@ -7,8 +7,9 @@ import (
 )
 
 type Config struct {
-	RequiredAlias map[string]string
-	Rules         []*Rule
+	RequiredAlias     map[string]string
+	Rules             []*Rule
+	DisallowUnaliased bool
 }
 
 func (c *Config) CompileRegexp() error {

flags.go

@@ -7,9 +7,10 @@ import (
 	"strings"
 )
 
-func flags() flag.FlagSet {
+func flags(config *Config) flag.FlagSet {
 	fs := flag.FlagSet{}
 	fs.Var(stringMap(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")
 	return fs
 }
 

testdata/src/a/a.go

@@ -2,10 +2,12 @@ package a
 
 import (
 	wrong_alias "fmt"      // want `import "fmt" imported as "wrong_alias" but must be "fff" according to config`
+	"io"
 	wrong_alias_again "os" // want `import "os" imported as "wrong_alias_again" but must be "stdos" according to config`
 )
 
 func ImportAsWrongAlias() {
 	wrong_alias.Println("foo")
 	wrong_alias_again.Stdout.WriteString("bar")
+	io.Pipe()
 }

testdata/src/a/a.go.golden

@@ -1,11 +1,13 @@
 package a
 
 import (
-	fff "fmt"  // want `import "fmt" imported as "wrong_alias" but must be "fff" according to config`
+	fff "fmt" // want `import "fmt" imported as "wrong_alias" but must be "fff" according to config`
+	"io"
 	stdos "os" // want `import "os" imported as "wrong_alias_again" but must be "stdos" according to config`
 )
 
 func ImportAsWrongAlias() {
 	fff.Println("foo")
 	stdos.Stdout.WriteString("bar")
+	io.Pipe()
 }

testdata/src/b/b.go

@@ -3,9 +3,11 @@ package b
 import (
 	fff "fmt"
 	stdos "os"
+	"io"
 )
 
 func ImportAsWrongAlias() {
 	fff.Println("foo")
 	stdos.Stdout.WriteString("bar")
+	io.Pipe()
 }

testdata/src/e/e.go

@@ -0,0 +1,13 @@
+package e
+
+import (
+	wrong_alias "fmt"      // want `import "fmt" imported as "wrong_alias" but must be "fff" according to config`
+	"io"                   // want `import "io" imported without alias but must be with alias "iio" according to config`
+	wrong_alias_again "os" // want `import "os" imported as "wrong_alias_again" but must be "stdos" according to config`
+)
+
+func ImportAsWrongAlias() {
+	wrong_alias.Println("foo")
+	wrong_alias_again.Stdout.WriteString("bar")
+	io.Pipe()
+}

testdata/src/e/e.go.golden

@@ -0,0 +1,13 @@
+package e
+
+import (
+	fff "fmt"  // want `import "fmt" imported as "wrong_alias" but must be "fff" according to config`
+	iio "io"   // want `import "io" imported without alias but must be with alias "iio" according to config`
+	stdos "os" // want `import "os" imported as "wrong_alias_again" but must be "stdos" according to config`
+)
+
+func ImportAsWrongAlias() {
+	fff.Println("foo")
+	stdos.Stdout.WriteString("bar")
+	iio.Pipe()
+}