From 9ac4766d76c2fed758ad8cd9d2699d9c469f4cf0 Mon Sep 17 00:00:00 2001
From: "Iskander (Alex) Sharipov" <quasilyte@gmail.com>
Date: Tue, 26 May 2020 23:06:32 +0300
Subject: [PATCH] checkers: add filepath.Join checker (#930)

Reports filepath.Join() string literal arguments that contain
'/' or '\' as it kinda defeats the purpose of using that function.

Signed-off-by: Iskander Sharipov <quasilyte@gmail.com>
---
 checkers/filepathJoin_checker.go              | 50 +++++++++++++++++++
 .../testdata/filepathJoin/negative_tests.go   | 16 ++++++
 .../testdata/filepathJoin/positive_tests.go   | 20 ++++++++
 3 files changed, 86 insertions(+)
 create mode 100644 checkers/filepathJoin_checker.go
 create mode 100644 checkers/testdata/filepathJoin/negative_tests.go
 create mode 100644 checkers/testdata/filepathJoin/positive_tests.go

diff --git a/checkers/filepathJoin_checker.go b/checkers/filepathJoin_checker.go
new file mode 100644
index 000000000..f51973e9c
--- /dev/null
+++ b/checkers/filepathJoin_checker.go
@@ -0,0 +1,50 @@
+package checkers
+
+import (
+	"go/ast"
+	"strings"
+
+	"github.com/go-lintpack/lintpack"
+	"github.com/go-lintpack/lintpack/astwalk"
+	"github.com/go-toolsmith/astcast"
+)
+
+func init() {
+	var info lintpack.CheckerInfo
+	info.Name = "filepathJoin"
+	info.Tags = []string{"diagnostic", "experimental"}
+	info.Summary = "Detects problems in filepath.Join() function calls"
+	info.Before = `filepath.Join("dir/", filename)`
+	info.After = `filepath.Join("dir", filename)`
+
+	collection.AddChecker(&info, func(ctx *lintpack.CheckerContext) lintpack.FileWalker {
+		return astwalk.WalkerForExpr(&filepathJoinChecker{ctx: ctx})
+	})
+}
+
+type filepathJoinChecker struct {
+	astwalk.WalkHandler
+	ctx *lintpack.CheckerContext
+}
+
+func (c *filepathJoinChecker) VisitExpr(expr ast.Expr) {
+	call := astcast.ToCallExpr(expr)
+	if qualifiedName(call.Fun) != "filepath.Join" {
+		return
+	}
+
+	for _, arg := range call.Args {
+		arg, ok := arg.(*ast.BasicLit)
+		if ok && c.hasSeparator(arg) {
+			c.warnSeparator(arg)
+		}
+	}
+}
+
+func (c *filepathJoinChecker) hasSeparator(v *ast.BasicLit) bool {
+	return strings.ContainsAny(v.Value, `/\`)
+}
+
+func (c *filepathJoinChecker) warnSeparator(sep ast.Expr) {
+	c.ctx.Warn(sep, "%s contains a path separator", sep)
+}
diff --git a/checkers/testdata/filepathJoin/negative_tests.go b/checkers/testdata/filepathJoin/negative_tests.go
new file mode 100644
index 000000000..bb07ceee3
--- /dev/null
+++ b/checkers/testdata/filepathJoin/negative_tests.go
@@ -0,0 +1,16 @@
+package checker_test
+
+import (
+	"path/filepath"
+)
+
+func badArgs() {
+	filename := "file.go"
+	dir := "testdata"
+
+	_ = filepath.Join("testdata", filename)
+
+	_ = filepath.Join(dir, "file.go")
+
+	_ = filepath.Join(`testdata`, `a`, `b.txt`)
+}
diff --git a/checkers/testdata/filepathJoin/positive_tests.go b/checkers/testdata/filepathJoin/positive_tests.go
new file mode 100644
index 000000000..523b1db74
--- /dev/null
+++ b/checkers/testdata/filepathJoin/positive_tests.go
@@ -0,0 +1,20 @@
+package checker_test
+
+import (
+	"path/filepath"
+)
+
+func badArgs() {
+	filename := "file.go"
+	dir := "testdata"
+
+	/*! "testdata/" contains a path separator */
+	_ = filepath.Join("testdata/", filename)
+
+	/*! "/file.go" contains a path separator */
+	_ = filepath.Join(dir, "/file.go")
+
+	/*! `\a\b.txt` contains a path separator */
+	/*! `testdata\` contains a path separator */
+	_ = filepath.Join(`testdata\`, `\a\b.txt`)
+}