Rename to nosprintfhostport

.github/workflows/go.yml

@@ -18,7 +18,7 @@ jobs:
       with:
         go-version: 1.17
 
-    - name: Build
+    - name: All
       run: make
 
     - name: Test

.gitignore

@@ -1 +1,3 @@
-gosprintfhostport.so
+nosprintfhostport.so
+nosprintfhostport
+

Makefile

@@ -1,13 +1,17 @@
-all: gosprintfhostport.so
+all: nosprintfhostport.so nosprintfhostport
 .PHONY: lint test
+
 clean:
-	rm gosprintfhostport.so
+	rm -f nosprintfhostport.so nosprintfhostport
 
 test:
 	go test ./...
 
 lint:
 	golangci-lint run ./...
 
-gosprintfhostport.so:
-	go build -buildmode=plugin plugin/gosprintfhostport.go
+nosprintfhostport:
+	go build ./cmd/nosprintfhostport
+
+nosprintfhostport.so:
+	go build -buildmode=plugin ./plugin/nosprintfhostport.go

cmd/go-sprintf-host-port/main.go → cmd/nosprintfhostport/main.go

@@ -3,7 +3,7 @@ package main
 import (
 	"golang.org/x/tools/go/analysis/singlechecker"
 
-	"github.com/stbenjam/go-sprintf-host-port/pkg/analyzer"
+	"github.com/stbenjam/no-sprintf-host-port/pkg/analyzer"
 )
 
 func main() {

go.mod

@@ -1,4 +1,4 @@
-module github.com/stbenjam/go-sprintf-host-port
+module github.com/stbenjam/no-sprintf-host-port
 
 go 1.16
 

pkg/analyzer/analyzer.go

@@ -1,6 +1,7 @@
 package analyzer
 
 import (
+	"fmt"
 	"go/ast"
 	"go/token"
 	"regexp"
@@ -12,8 +13,8 @@ import (
 )
 
 var Analyzer = &analysis.Analyzer{
-	Name:     "gosprintfhostport",
-	Doc:      "Checks that sprintf is not used to construct a host:port combination in a URL.",
+	Name:     "nosprintfhostport",
+	Doc:      "Checks for misuse of Sprintf to construct a host with port in a URL.",
 	Run:      run,
 	Requires: []*analysis.Analyzer{inspect.Analyzer},
 }
@@ -26,53 +27,70 @@ func run(pass *analysis.Pass) (interface{}, error) {
 
 	inspector.Preorder(nodeFilter, func(node ast.Node) {
 		callExpr := node.(*ast.CallExpr)
-
-		selector, ok := callExpr.Fun.(*ast.SelectorExpr)
-		if !ok {
-			return
-		}
-		pkg, ok := selector.X.(*ast.Ident)
-		if !ok {
-			return
-		}
-		if pkg.Name != "fmt" || selector.Sel.Name != "Sprintf" {
-			return
+		if p, f, ok := getCallExprFunction(callExpr); ok && p == "fmt" && f == "Sprintf" {
+			if err := checkForHostPortConstruction(callExpr); err != nil {
+				pass.Reportf(node.Pos(), err.Error())
+			}
 		}
+	})
 
-		if len(callExpr.Args) < 2 {
-			return
-		}
+	return nil, nil
+}
 
-		// Let's see if our format string is a string literal.
-		fsRaw, ok := callExpr.Args[0].(*ast.BasicLit)
-		if !ok {
-			return
-		}
-		if fsRaw.Kind != token.STRING {
-			return
-		}
+// getCallExprFunction returns the package and function name from a callExpr, if any.
+func getCallExprFunction(callExpr *ast.CallExpr) (pkg string, fn string, result bool) {
+	selector, ok := callExpr.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return "", "", false
+	}
+	gopkg, ok := selector.X.(*ast.Ident)
+	if !ok {
+		return "", "", false
+	}
+	return gopkg.Name, selector.Sel.Name, true
+}
 
-		// Remove quotes
-		fs := fsRaw.Value[1 : len(fsRaw.Value)-1]
+// getStringLiteral returns the value at a position if it's a string literal.
+func getStringLiteral(args []ast.Expr, pos int) (string, bool) {
+	if len(args) < pos + 1 {
+		return "", false
+	}
 
-		regexes := []*regexp.Regexp{
-			// These check to see if it looks like a URI with a port, basically scheme://%s:<something else>,
-			// or scheme://user:pass@%s:<something else>.
-			// Matching requirements:
-			//		- Scheme as per RFC3986 is ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
-			//		- A format string substitution in the host portion, preceded by an optional username/password@
-			//  	- A colon indicating a port will be specified
-			regexp.MustCompile(`^[a-zA-Z0-9+-.]*://%s:[^@]*$`),
-			regexp.MustCompile(`^[a-zA-Z0-9+-.]*://[^/]*@%s:.*$`),
-		}
+	// Let's see if our format string is a string literal.
+	fsRaw, ok := args[pos].(*ast.BasicLit)
+	if !ok {
+		return "", false
+	}
+	if fsRaw.Kind == token.STRING && len(fsRaw.Value) >= 2 {
+		return fsRaw.Value[1 : len(fsRaw.Value)-1], true
+	} else {
+		return "", false
+	}
+}
 
-		for _, re := range regexes {
-			if re.MatchString(fs) {
-				pass.Reportf(node.Pos(), "host:port in url should be constructed with net.JoinHostPort and not directly with fmt.Sprintf")
-				break
-			}
+// checkForHostPortConstruction checks to see if a sprintf call looks like a URI with a port,
+// essentially scheme://%s:<something else>, or scheme://user:pass@%s:<something else>.
+//
+// Matching requirements:
+//		- Scheme as per RFC3986 is ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
+//		- A format string substitution in the host portion, preceded by an optional username/password@
+//  	- A colon indicating a port will be specified
+func checkForHostPortConstruction(sprintf *ast.CallExpr) error {
+	fs, ok := getStringLiteral(sprintf.Args, 0)
+	if !ok {
+		return nil
+	}
+
+	regexes := []*regexp.Regexp{
+		regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+-.]*://%s:[^@]*$`),    // URL without basic auth user
+		regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+-.]*://[^/]*@%s:.*$`), // URL with basic auth
+	}
+
+	for _, re := range regexes {
+		if re.MatchString(fs) {
+			return fmt.Errorf("host:port in url should be constructed with net.JoinHostPort and not directly with fmt.Sprintf")
 		}
-	})
+	}
 
-	return nil, nil
+	return nil
 }

plugin/gosprintfhostport.go → plugin/nosprintfhostport.go

@@ -3,7 +3,7 @@ package main
 import (
 	"golang.org/x/tools/go/analysis"
 
-	"github.com/stbenjam/go-sprintf-host-port/pkg/analyzer"
+	"github.com/stbenjam/no-sprintf-host-port/pkg/analyzer"
 )
 
 type analyzerPlugin struct{}

testdata/src/p/p.go

@@ -6,15 +6,18 @@ import (
 )
 
 func _() {
-	_ = fmt.Sprintf("gopher://%s/foo", net.JoinHostPort("foo", "80"))
 
 	_ = fmt.Sprintf("postgres://%s:%[email protected]/%s", "foo", "bar", "baz")
 
-	_ = fmt.Sprintf("http://%s/foo", net.JoinHostPort("foo", "80"))
+	_ = fmt.Sprintf("http://api.%s/foo", "example.com")
 
 	_ = fmt.Sprintf("http://api.%s:6443/foo", "example.com")
 
-	_ = fmt.Sprintf("http://api.%s/foo", "example.com")
+	_ = fmt.Sprintf("http://%s/foo", net.JoinHostPort("foo", "80"))
+
+	_ = fmt.Sprintf("9invalidscheme://%s:%d", "myHost", 70)
+
+	_ = fmt.Sprintf("gopher://%s/foo", net.JoinHostPort("foo", "80"))
 
 	_ = fmt.Sprintf("telnet+ssl://%s/foo", net.JoinHostPort("foo", "80"))
 
@@ -28,6 +31,8 @@ func _() {
 
 	_ = fmt.Sprintf("telnet+ssl://%s:%d", "myHost", 23) // want "should be constructed with net.JoinHostPort"
 
+	_ = fmt.Sprintf("weird3.6://%s:%d", "myHost", 23) // want "should be constructed with net.JoinHostPort"
+
 	_ = fmt.Sprintf("https://user@%s:%d", "myHost", 8443) // want "should be constructed with net.JoinHostPort"
 
 	_ = fmt.Sprintf("postgres://%s:%s@%s:5050/%s", "foo", "bar", "baz", "qux") // want "should be constructed with net.JoinHostPort"