Merge pull request #5 from george-maroun/issue#3/detect-NewLogger

Only check for missing traceId on WithValues called on NewLogger

cover.out

@@ -1,4 +1,8 @@
 mode: atomic
+github.com/george-maroun/tracecheck/internal/stringutil/is.go:6.29,7.22 1 4
+github.com/george-maroun/tracecheck/internal/stringutil/is.go:7.22,8.25 1 9
+github.com/george-maroun/tracecheck/internal/stringutil/is.go:8.25,11.4 1 2
+github.com/george-maroun/tracecheck/internal/stringutil/is.go:14.2,14.13 1 2
 github.com/george-maroun/tracecheck/internal/bytebufferpool/pool.go:9.26,11.3 1 1
 github.com/george-maroun/tracecheck/internal/bytebufferpool/pool.go:14.26,18.2 3 1
 github.com/george-maroun/tracecheck/internal/bytebufferpool/pool.go:20.29,22.2 1 1
@@ -76,10 +80,6 @@ github.com/george-maroun/tracecheck/internal/sets/string.go:43.41,45.13 2 3
 github.com/george-maroun/tracecheck/internal/sets/string.go:45.13,48.3 2 1
 github.com/george-maroun/tracecheck/internal/sets/string.go:50.2,53.12 4 2
 github.com/george-maroun/tracecheck/internal/sets/string.go:57.36,59.2 1 4
-github.com/george-maroun/tracecheck/internal/stringutil/is.go:6.29,7.22 1 4
-github.com/george-maroun/tracecheck/internal/stringutil/is.go:7.22,8.25 1 9
-github.com/george-maroun/tracecheck/internal/stringutil/is.go:8.25,11.4 1 2
-github.com/george-maroun/tracecheck/internal/stringutil/is.go:14.2,14.13 1 2
 github.com/george-maroun/tracecheck/loggercheck.go:24.53,34.2 3 2
 github.com/george-maroun/tracecheck/loggercheck.go:51.50,66.27 7 2
 github.com/george-maroun/tracecheck/loggercheck.go:66.27,68.3 1 0
@@ -101,10 +101,10 @@ github.com/george-maroun/tracecheck/loggercheck.go:112.3,113.21 2 3
 github.com/george-maroun/tracecheck/loggercheck.go:113.21,115.4 1 3
 github.com/george-maroun/tracecheck/loggercheck.go:116.3,116.17 1 0
 github.com/george-maroun/tracecheck/loggercheck.go:119.2,119.12 1 2
-github.com/george-maroun/tracecheck/loggercheck.go:122.85,124.15 2 14
+github.com/george-maroun/tracecheck/loggercheck.go:122.85,124.15 2 13
 github.com/george-maroun/tracecheck/loggercheck.go:124.15,126.3 1 1
-github.com/george-maroun/tracecheck/loggercheck.go:128.2,129.28 2 13
-github.com/george-maroun/tracecheck/loggercheck.go:129.28,131.3 1 7
+github.com/george-maroun/tracecheck/loggercheck.go:128.2,129.28 2 12
+github.com/george-maroun/tracecheck/loggercheck.go:129.28,131.3 1 6
 github.com/george-maroun/tracecheck/loggercheck.go:134.2,134.29 1 6
 github.com/george-maroun/tracecheck/loggercheck.go:134.29,136.3 1 1
 github.com/george-maroun/tracecheck/loggercheck.go:138.2,139.20 2 5
@@ -126,12 +126,17 @@ github.com/george-maroun/tracecheck/loggercheck.go:185.2,187.12 2 2
 github.com/george-maroun/tracecheck/loggercheck.go:190.69,192.16 2 2
 github.com/george-maroun/tracecheck/loggercheck.go:192.16,194.3 1 0
 github.com/george-maroun/tracecheck/loggercheck.go:196.2,200.48 3 2
-github.com/george-maroun/tracecheck/loggercheck.go:200.48,205.18 3 14
-github.com/george-maroun/tracecheck/loggercheck.go:205.18,207.4 1 0
-github.com/george-maroun/tracecheck/loggercheck.go:212.3,218.17 5 14
-github.com/george-maroun/tracecheck/loggercheck.go:218.17,221.4 1 0
-github.com/george-maroun/tracecheck/loggercheck.go:223.3,223.37 1 14
-github.com/george-maroun/tracecheck/loggercheck.go:226.2,226.17 1 2
+github.com/george-maroun/tracecheck/loggercheck.go:200.48,204.21 3 13
+github.com/george-maroun/tracecheck/loggercheck.go:204.21,206.4 1 0
+github.com/george-maroun/tracecheck/loggercheck.go:209.3,210.32 2 13
+github.com/george-maroun/tracecheck/loggercheck.go:210.32,211.42 1 13
+github.com/george-maroun/tracecheck/loggercheck.go:211.42,213.10 2 13
+github.com/george-maroun/tracecheck/loggercheck.go:217.3,217.18 1 13
+github.com/george-maroun/tracecheck/loggercheck.go:217.18,219.4 1 0
+github.com/george-maroun/tracecheck/loggercheck.go:223.3,229.17 5 13
+github.com/george-maroun/tracecheck/loggercheck.go:229.17,232.4 1 0
+github.com/george-maroun/tracecheck/loggercheck.go:234.3,234.37 1 13
+github.com/george-maroun/tracecheck/loggercheck.go:237.2,237.17 1 2
 github.com/george-maroun/tracecheck/options.go:9.43,10.30 1 0
 github.com/george-maroun/tracecheck/options.go:10.30,12.3 1 0
 github.com/george-maroun/tracecheck/options.go:15.45,16.30 1 0
@@ -154,67 +159,81 @@ github.com/george-maroun/tracecheck/internal/checkers/checker.go:32.83,39.27 6 3
 github.com/george-maroun/tracecheck/internal/checkers/checker.go:39.27,41.3 1 0
 github.com/george-maroun/tracecheck/internal/checkers/checker.go:43.2,45.31 2 3
 github.com/george-maroun/tracecheck/internal/checkers/checker.go:45.31,54.3 3 1
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:56.2,57.45 2 3
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:57.45,60.10 3 4
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:60.10,62.12 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:74.3,74.35 1 4
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:74.35,76.9 2 1
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:80.2,80.17 1 3
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:80.17,92.17 3 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:92.17,100.4 2 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:103.3,113.28 5 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:113.28,115.4 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:117.3,121.37 3 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:121.37,123.4 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:126.3,150.39 7 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:150.39,159.4 2 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:161.3,168.18 2 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:171.2,171.26 1 3
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:171.26,173.3 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:175.2,175.22 1 3
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:175.22,178.3 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:183.92,184.42 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:184.42,185.24 1 312
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:186.22,187.67 1 4
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:187.67,189.5 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:190.21,191.50 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:191.50,193.5 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:195.3,195.14 1 312
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:197.2,197.8 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:202.70,205.35 2 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:205.35,207.29 2 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:207.29,209.4 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:210.3,210.19 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:214.2,214.20 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:218.52,222.32 3 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:222.32,225.58 2 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:225.58,227.5 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:228.4,228.26 1 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:231.2,231.18 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:235.50,236.27 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:236.27,238.6 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:240.5,240.23 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:243.53,244.27 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:244.27,246.6 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:248.5,248.23 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:254.80,255.20 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:255.20,257.6 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:259.5,259.37 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:259.37,261.16 2 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:261.16,262.21 1 4
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:266.9,266.40 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:266.40,267.48 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:267.48,269.23 2 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:269.23,271.18 1 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:275.5,275.28 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:280.66,282.16 2 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:282.16,284.3 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:286.2,286.41 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:286.41,287.56 1 6
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:287.56,289.5 1 0
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:292.2,292.26 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:292.26,295.3 1 2
-github.com/george-maroun/tracecheck/internal/checkers/checker.go:297.2,297.25 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:57.2,58.21 2 3
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:58.21,60.3 1 2
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:62.2,63.45 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:63.45,66.10 3 2
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:66.10,68.12 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:80.3,80.35 1 2
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:80.35,82.9 2 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:86.2,86.17 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:86.17,98.17 3 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:98.17,106.4 2 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:109.3,119.28 5 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:119.28,121.4 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:123.3,127.37 3 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:127.37,129.4 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:132.3,156.39 7 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:156.39,165.4 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:167.3,174.18 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:177.2,177.26 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:177.26,179.3 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:181.2,181.22 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:181.22,184.3 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:189.92,190.42 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:190.42,191.24 1 126
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:192.22,193.67 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:193.67,195.5 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:196.21,197.50 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:197.50,199.5 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:201.3,201.14 1 126
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:203.2,203.8 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:208.70,211.35 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:211.35,213.29 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:213.29,215.4 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:216.3,216.19 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:220.2,220.20 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:224.52,228.32 3 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:228.32,231.58 2 4
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:231.58,233.5 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:234.4,234.26 1 4
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:237.2,237.18 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:241.50,242.27 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:242.27,244.6 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:246.5,246.23 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:249.53,250.27 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:250.27,252.6 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:254.5,254.23 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:260.80,261.20 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:261.20,263.6 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:265.5,265.37 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:265.37,267.16 2 2
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:267.16,268.21 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:272.9,272.40 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:272.40,273.48 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:273.48,275.23 2 3
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:275.23,277.18 1 3
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:281.5,281.28 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:286.66,288.16 2 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:288.16,290.3 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:292.2,292.41 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:292.41,293.56 1 3
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:293.56,295.5 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:298.2,298.26 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:298.26,301.3 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:303.2,303.25 1 0
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:307.70,310.42 2 3
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:310.42,311.15 1 416
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:311.15,313.4 1 208
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:315.3,315.39 1 208
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:315.39,317.41 1 23
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:317.41,319.52 1 4
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:319.52,321.47 1 4
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:321.47,322.54 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:322.54,323.39 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:323.39,325.9 1 1
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:331.3,331.14 1 208
+github.com/george-maroun/tracecheck/internal/checkers/checker.go:333.2,333.15 1 3
 github.com/george-maroun/tracecheck/internal/checkers/common.go:21.91,22.55 1 0
 github.com/george-maroun/tracecheck/internal/checkers/common.go:22.55,23.113 1 0
 github.com/george-maroun/tracecheck/internal/checkers/common.go:23.113,25.4 1 0

internal/checkers/checker.go

@@ -52,9 +52,14 @@ func ExecuteChecker(c Checker, pass *analysis.Pass, call CallContext, cfg Config
 			Message:  "odd number of arguments passed as key-value pairs for logging",
 		})
 	}
-	// TODO: VERY IMPORTANT 🚨 should initialize hasTraceId to false
-	// hasTraceId is initialized to true to temporarily turn off traceId check 
-	hasTraceId := true
+
+	// Return if WithValues is not invoked on NewLogger
+	result := isWithValuesCallOnNewLogger(call.File, call.Expr.Pos())
+	if result == false {
+		return
+	}
+
+	hasTraceId := false
 	for i := 0; i < len(keyValuesArgs); i += 2 {
 		arg := keyValuesArgs[i]
 		basic, ok := arg.(*ast.BasicLit)
@@ -297,3 +302,38 @@ func getImportPos(file *ast.File, lib string) (token.Pos, error) {
 
 	return token.NoPos, nil
 }
+
+// TODO: Refactor this function
+func isWithValuesCallOnNewLogger(file *ast.File, pos token.Pos) bool {
+	// Function to find the node at the given position
+	var result bool
+	ast.Inspect(file, func(n ast.Node) bool {
+		if n == nil {
+			return false
+		}
+		// Check if the node corresponds to the given position
+		if n.Pos() <= pos && pos <= n.End() {
+			// Check if the node is a call expression
+			if call, ok := n.(*ast.CallExpr); ok {
+				// Check if the function being called is a selector expression (e.g., obj.Method())
+				if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
+						// Check if the receiver is a call to zapr.NewLogger
+					if call, ok := sel.X.(*ast.CallExpr); ok {
+						if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
+							if sel.Sel.Name == "NewLogger" {
+								result = true
+							}
+						}
+					}
+				}
+			}
+		}
+		return true
+	})
+	return result
+}
+
+
+
+
+

loggercheck.go

@@ -200,19 +200,30 @@ func (l *loggercheck) run(pass *analysis.Pass) (interface{}, error) {
 	insp.Preorder(nodeFilter, func(node ast.Node) {
 		call := node.(*ast.CallExpr)
 
-		// Get the current file from the node's position
-		file := pass.Fset.File(call.Pos())
+		tokFile := pass.Fset.File(call.Pos())
+		if tokFile == nil {
+			return
+		}
+
+		// Find the corresponding ast.File
+		var file *ast.File
+		for _, f := range pass.Files {
+			if pass.Fset.File(f.Pos()) == tokFile {
+				file = f
+				break
+			}
+		}
+
 		if file == nil {
 			return
 		}
 
 		// Save the current file to the map
-		// N.B.: Not sure if correct way to get current file
     // Lock the mutex before accessing the shared resource
 		l.mu.Lock()
-		l.CallToFile[call] = pass.Files[0]
+		l.CallToFile[call] = file
 		// Unlock it afterwards
-		l.mu.Unlock()
+		defer l.mu.Unlock()
 
 		typ := pass.TypesInfo.Types[call.Fun].Type
 		if typ == nil {

testdata/src/a/all/example.go

@@ -6,11 +6,6 @@ import (
 	"github.com/go-logr/logr"
 )
 
-func ExampleLogRTraceID() {
-	log := logr.Discard()
-	log = log.WithValues("key", "value")   // want `missing traceId in logging keys`
-}
-
 func ExampleInvalid() {
 	// function pointer is not supported
 	log := logr.Discard()

testdata/src/a/fix_import/example.go

@@ -8,6 +8,7 @@ import (
 
 func SomeFunc(ctx context.Context, eventType, deliveryID string, payload []byte) error {
 	log := zapr.NewLogger(zap.L()).WithValues("eventType", eventType, "deliverID", deliveryID)  // want `missing traceId in logging keys`
+	log = log.WithValues("eventType", "hello")
 	log.Info("Tracing")
 	return nil
 }

testdata/src/a/fix_import/example.go.golden

@@ -10,6 +10,7 @@ import (
 func SomeFunc(ctx context.Context, eventType, deliveryID string, payload []byte) error {
 	span := trace.SpanFromContext(ctx)
 	log := zapr.NewLogger(zap.L()).WithValues("traceId", span.SpanContext().TraceID().String(), "spanId", span.SpanContext().SpanID().String(), "eventType", eventType, "deliverID", deliveryID) // want `missing traceId in logging keys`
+	log = log.WithValues("eventType", "hello")
 	log.Info("Tracing")
 	return nil
 }