Make the warning about 'empty' more specific

rillig · Mar 5, 2024 · c7de1e8 · c7de1e8
1 parent 0bee63d
commit c7de1e8
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 17 deletions.
diff --git a/v23/mkcondchecker.go b/v23/mkcondchecker.go
@@ -138,31 +138,46 @@ func (ck *MkCondChecker) checkNotEmpty(not *MkCond) {
 // checkEmpty checks a condition of the form empty(VAR),
 // empty(VAR:Mpattern) or ${VAR:Mpattern} in an .if directive.
 func (ck *MkCondChecker) checkEmpty(expr *MkExpr, fromEmpty bool, neg bool) {
-	ck.checkEmptyExpr(expr)
+	ck.checkEmptyExpr(expr, neg)
 	ck.checkEmptyType(expr)
 
 	s := MkCondSimplifier{ck.MkLines, ck.MkLine}
 	s.SimplifyExpr(expr, fromEmpty, neg)
 }
 
-func (ck *MkCondChecker) checkEmptyExpr(expr *MkExpr) {
+func (ck *MkCondChecker) checkEmptyExpr(expr *MkExpr, neg bool) {
 	if !matches(expr.varname, `^\$.*:[MN]`) {
 		return
 	}
 
 	ck.MkLine.Warnf("The empty() function takes a variable name as parameter, " +
-		"not a variable expression.")
-	ck.MkLine.Explain(
-		"Instead of empty(${VARNAME:Mpattern}), you should write either of the following:",
-		"",
-		"\tempty(VARNAME:Mpattern)",
-		"\t${VARNAME:Mpattern} == \"\"",
-		"\t!${VARNAME:Mpattern}",
-		"",
-		"Instead of !empty(${VARNAME:Mpattern}), you should write either of the following:",
-		"",
-		"\t!empty(VARNAME:Mpattern)",
-		"\t${VARNAME:Mpattern}")
+		"not an expression.")
+	if neg {
+		ck.MkLine.Explain(
+			"Instead of !empty(${VARNAME:Mpattern}),",
+			"you should write either of the following:",
+			"",
+			"\t!empty(VARNAME:Mpattern)",
+			"\t${VARNAME:Mpattern} != \"\"",
+			"",
+			"If the pattern cannot match the number zero,",
+			"you can omit the '!= \"\"', resulting in:",
+			"",
+			"\t${VARNAME:Mpattern}")
+	} else {
+		ck.MkLine.Explain(
+			"Instead of empty(${VARNAME:Mpattern}),",
+			"you should write either of the following:",
+			"",
+			"\tempty(VARNAME:Mpattern)",
+			"\t${VARNAME:Mpattern} == \"\"",
+			"",
+			"If the pattern cannot match the number zero,",
+			"you can replace the '== \"\"' with a '!',",
+			"resulting in:",
+			"",
+			"\t!${VARNAME:Mpattern}")
+	}
 }
 
 func (ck *MkCondChecker) checkEmptyType(expr *MkExpr) {

diff --git a/v23/mkcondchecker_test.go b/v23/mkcondchecker_test.go
@@ -59,7 +59,7 @@ func (s *Suite) Test_MkCondChecker_Check(c *check.C) {
 
 	test(".if !empty(${IS_BUILTIN.Xfixes:M[yY][eE][sS]})",
 		"WARN: filename.mk:4: The empty() function takes a variable name as parameter, "+
-			"not a variable expression.")
+			"not an expression.")
 
 	test(".if ${PKGSRC_COMPILER} == \"msvc\"",
 		"WARN: filename.mk:4: \"msvc\" is not valid for PKGSRC_COMPILER. "+
@@ -578,7 +578,7 @@ func (s *Suite) Test_MkCondChecker_checkEmptyExpr(c *check.C) {
 			"# dummy")
 		ck := NewMkCondChecker(mklines.mklines[0], mklines)
 
-		ck.checkEmptyExpr(NewMkExpr(expr))
+		ck.checkEmptyExpr(NewMkExpr(expr), false)
 
 		t.CheckOutput(diagnostics)
 	}
@@ -605,7 +605,7 @@ func (s *Suite) Test_MkCondChecker_checkEmptyExpr(c *check.C) {
 	test(
 		"${PREFIX:Mpattern}",
 		"WARN: filename.mk:1: The empty() function takes a variable "+
-			"name as parameter, not a variable expression.")
+			"name as parameter, not an expression.")
 }
 
 func (s *Suite) Test_MkCondChecker_checkEmptyType(c *check.C) {