pkgsrc-Changes archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
CVS commit: pkgsrc/pkgtools/pkglint
Module Name: pkgsrc
Committed By: rillig
Date: Sun Jun 14 11:35:54 UTC 2020
Modified Files:
pkgsrc/pkgtools/pkglint: Makefile PLIST
pkgsrc/pkgtools/pkglint/files: check_test.go mkcondchecker.go
mkcondchecker_test.go mkline.go mkparser.go mkparser_test.go
util.go util_test.go
Added Files:
pkgsrc/pkgtools/pkglint/files: scope.go scope_test.go
pkgsrc/pkgtools/pkglint/files/makepat: pat.go pat_test.go
Log Message:
pkgtools/pkglint: update to 20.1.17
Changes since 20.1.16:
Conditions that contradict each other in the same file are reported as
errors. Inspired by lang/rust/Makefile r1.174.
To generate a diff of this commit:
cvs rdiff -u -r1.655 -r1.656 pkgsrc/pkgtools/pkglint/Makefile
cvs rdiff -u -r1.26 -r1.27 pkgsrc/pkgtools/pkglint/PLIST
cvs rdiff -u -r1.72 -r1.73 pkgsrc/pkgtools/pkglint/files/check_test.go
cvs rdiff -u -r1.8 -r1.9 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go \
pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
cvs rdiff -u -r1.80 -r1.81 pkgsrc/pkgtools/pkglint/files/mkline.go \
pkgsrc/pkgtools/pkglint/files/util.go
cvs rdiff -u -r1.42 -r1.43 pkgsrc/pkgtools/pkglint/files/mkparser.go
cvs rdiff -u -r1.39 -r1.40 pkgsrc/pkgtools/pkglint/files/mkparser_test.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/scope.go \
pkgsrc/pkgtools/pkglint/files/scope_test.go
cvs rdiff -u -r1.54 -r1.55 pkgsrc/pkgtools/pkglint/files/util_test.go
cvs rdiff -u -r0 -r1.1 pkgsrc/pkgtools/pkglint/files/makepat/pat.go \
pkgsrc/pkgtools/pkglint/files/makepat/pat_test.go
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: pkgsrc/pkgtools/pkglint/Makefile
diff -u pkgsrc/pkgtools/pkglint/Makefile:1.655 pkgsrc/pkgtools/pkglint/Makefile:1.656
--- pkgsrc/pkgtools/pkglint/Makefile:1.655 Fri Jun 12 19:14:45 2020
+++ pkgsrc/pkgtools/pkglint/Makefile Sun Jun 14 11:35:54 2020
@@ -1,6 +1,6 @@
-# $NetBSD: Makefile,v 1.655 2020/06/12 19:14:45 rillig Exp $
+# $NetBSD: Makefile,v 1.656 2020/06/14 11:35:54 rillig Exp $
-PKGNAME= pkglint-20.1.16
+PKGNAME= pkglint-20.1.17
CATEGORIES= pkgtools
DISTNAME= tools
MASTER_SITES= ${MASTER_SITE_GITHUB:=golang/}
Index: pkgsrc/pkgtools/pkglint/PLIST
diff -u pkgsrc/pkgtools/pkglint/PLIST:1.26 pkgsrc/pkgtools/pkglint/PLIST:1.27
--- pkgsrc/pkgtools/pkglint/PLIST:1.26 Sun Jan 26 17:12:36 2020
+++ pkgsrc/pkgtools/pkglint/PLIST Sun Jun 14 11:35:54 2020
@@ -1,10 +1,11 @@
-@comment $NetBSD: PLIST,v 1.26 2020/01/26 17:12:36 rillig Exp $
+@comment $NetBSD: PLIST,v 1.27 2020/06/14 11:35:54 rillig Exp $
bin/pkglint
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/getopt.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/histogram.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/intqa.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/licenses.a
+gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/makepat.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/pkgver.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/regex.a
gopkg/pkg/${GO_PLATFORM}/netbsd.org/pkglint/textproc.a
@@ -50,6 +51,8 @@ gopkg/src/netbsd.org/pkglint/lineslexer.
gopkg/src/netbsd.org/pkglint/lineslexer_test.go
gopkg/src/netbsd.org/pkglint/logging.go
gopkg/src/netbsd.org/pkglint/logging_test.go
+gopkg/src/netbsd.org/pkglint/makepat/pat.go
+gopkg/src/netbsd.org/pkglint/makepat/pat_test.go
gopkg/src/netbsd.org/pkglint/mkassignchecker.go
gopkg/src/netbsd.org/pkglint/mkassignchecker_test.go
gopkg/src/netbsd.org/pkglint/mkcondchecker.go
@@ -101,6 +104,8 @@ gopkg/src/netbsd.org/pkglint/plist_test.
gopkg/src/netbsd.org/pkglint/redundantscope.go
gopkg/src/netbsd.org/pkglint/redundantscope_test.go
gopkg/src/netbsd.org/pkglint/regex/regex.go
+gopkg/src/netbsd.org/pkglint/scope.go
+gopkg/src/netbsd.org/pkglint/scope_test.go
gopkg/src/netbsd.org/pkglint/shell.go
gopkg/src/netbsd.org/pkglint/shell.y
gopkg/src/netbsd.org/pkglint/shell_test.go
Index: pkgsrc/pkgtools/pkglint/files/check_test.go
diff -u pkgsrc/pkgtools/pkglint/files/check_test.go:1.72 pkgsrc/pkgtools/pkglint/files/check_test.go:1.73
--- pkgsrc/pkgtools/pkglint/files/check_test.go:1.72 Sun Jun 7 15:49:23 2020
+++ pkgsrc/pkgtools/pkglint/files/check_test.go Sun Jun 14 11:35:54 2020
@@ -128,6 +128,7 @@ func Test__qa(t *testing.T) {
ck.Configure("pkglint.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("pkgsrc.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("redundantscope.go", "*", "*", -intqa.EMissingTest) // TODO
+ ck.Configure("scope.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("shell.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("shtokenizer.go", "*", "*", -intqa.EMissingTest) // TODO
ck.Configure("shtypes.go", "*", "*", -intqa.EMissingTest) // TODO
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.8 pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.9
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker.go:1.8 Sun Jun 7 15:49:23 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker.go Sun Jun 14 11:35:54 2020
@@ -1,6 +1,9 @@
package pkglint
-import "netbsd.org/pkglint/textproc"
+import (
+ "netbsd.org/pkglint/makepat"
+ "netbsd.org/pkglint/textproc"
+)
// MkCondChecker checks conditions in Makefiles.
// These conditions occur in .if and .elif clauses, as well as the
@@ -72,6 +75,8 @@ func (ck *MkCondChecker) Check() {
Var: checkVar,
Compare: ck.checkCompare,
VarUse: checkVarUse})
+
+ ck.checkContradictions()
}
func (ck *MkCondChecker) checkNotEmpty(not *MkCond) {
@@ -361,3 +366,92 @@ func (ck *MkCondChecker) checkNotCompare
ck.MkLine.Warnf("The ! should use parentheses or be merged into the comparison operator.")
}
+
+func (ck *MkCondChecker) checkContradictions() {
+ mkline := ck.MkLine
+
+ byVarname := make(map[string][]VarFact)
+ levels := ck.MkLines.indentation.levels
+ for _, level := range levels[:len(levels)-1] {
+ if !level.mkline.NeedsCond() {
+ continue
+ }
+ prevFacts := ck.collectFacts(level.mkline)
+ for _, prevFact := range prevFacts {
+ varname := prevFact.Varname
+ byVarname[varname] = append(byVarname[varname], prevFact)
+ }
+ }
+
+ facts := ck.collectFacts(mkline)
+ for _, curr := range facts {
+ varname := curr.Varname
+ for _, prev := range byVarname[varname] {
+ both := makepat.Intersect(prev.Matches, curr.Matches)
+ if !both.CanMatch() {
+ if prev.MkLine != mkline {
+ mkline.Errorf("The patterns %q from %s and %q cannot match at the same time.",
+ prev.Text, mkline.RelMkLine(prev.MkLine), curr.Text)
+ } else {
+ mkline.Errorf("The patterns %q and %q cannot match at the same time.",
+ prev.Text, curr.Text)
+ }
+ }
+ }
+ byVarname[varname] = append(byVarname[varname], curr)
+ }
+}
+
+type VarFact struct {
+ MkLine *MkLine
+ Varname string
+ Text string
+ Matches *makepat.Pattern
+}
+
+func (ck *MkCondChecker) collectFacts(mkline *MkLine) []VarFact {
+ var facts []VarFact
+
+ collectUse := func(use *MkVarUse) {
+ if use == nil || len(use.modifiers) != 1 {
+ return
+ }
+
+ ok, positive, pattern, _ := use.modifiers[0].MatchMatch()
+ if !ok || !positive || containsVarUse(pattern) {
+ return
+ }
+
+ vartype := G.Pkgsrc.VariableType(ck.MkLines, use.varname)
+ if vartype == nil || vartype.IsList() {
+ return
+ }
+
+ m, err := makepat.Compile(pattern)
+ if err != nil {
+ return
+ }
+
+ facts = append(facts, VarFact{mkline, use.varname, pattern, m})
+ }
+
+ var collectCond func(cond *MkCond)
+ collectCond = func(cond *MkCond) {
+ if cond.Term != nil {
+ collectUse(cond.Term.Var)
+ }
+ if cond.Not != nil {
+ collectUse(cond.Not.Empty)
+ }
+ for _, cond := range cond.And {
+ collectCond(cond)
+ }
+ if cond.Paren != nil {
+ collectCond(cond.Paren)
+ }
+ }
+
+ collectCond(mkline.Cond())
+
+ return facts
+}
Index: pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.8 pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.9
--- pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go:1.8 Tue Jun 2 17:52:26 2020
+++ pkgsrc/pkgtools/pkglint/files/mkcondchecker_test.go Sun Jun 14 11:35:54 2020
@@ -216,6 +216,139 @@ func (s *Suite) Test_MkCondChecker_Check
"ERROR: Makefile:6: Use ${PKGSRC_COMPILER:Ngcc} instead of the != operator.")
}
+func (s *Suite) Test_MkCondChecker_Check__contradicting_conditions(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPkgsrc()
+ t.FinishSetUp()
+
+ lines := func(lines ...string) []string { return lines }
+ test := func(lines []string, diagnostics ...string) {
+ allLines := []string{
+ MkCvsID,
+ "",
+ ".include \"../../mk/bsd.prefs.mk\"",
+ ""}
+ mklines := t.NewMkLines("filename.mk",
+ append(allLines, lines...)...)
+
+ mklines.Check()
+
+ t.CheckOutput(diagnostics)
+ }
+
+ // Seen in lang/rust/Makefile on 2020-06-12.
+ // TODO: The MACHINE_PLATFORM conditions make the OPSYS condition redundant.
+ test(
+ lines(
+ ".if ${OPSYS} == \"NetBSD\" && "+
+ "!empty(MACHINE_PLATFORM:MNetBSD-9.99.*) && "+
+ "!empty(MACHINE_PLATFORM:MNetBSD-[1-9][0-9].*)",
+ ".endif"),
+ "ERROR: filename.mk:5: The patterns \"NetBSD-9.99.*\" "+
+ "and \"NetBSD-[1-9][0-9].*\" cannot match at the same time.")
+
+ // A syntactical variation of the above condition.
+ // TODO: The MACHINE_PLATFORM conditions make the OPSYS condition redundant.
+ test(
+ lines(
+ ".if ${OPSYS} == NetBSD && ${MACHINE_PLATFORM:MNetBSD-9.99.*} && ${MACHINE_PLATFORM:MNetBSD-[1-9][0-9].*}",
+ ".endif"),
+ "ERROR: filename.mk:5: The patterns \"NetBSD-9.99.*\" "+
+ "and \"NetBSD-[1-9][0-9].*\" cannot match at the same time.")
+
+ // Another variation on the same theme.
+ // TODO: The MACHINE_PLATFORM conditions make the OPSYS condition redundant.
+ test(
+ lines(
+ ".if ${OPSYS} == NetBSD",
+ ". if ${MACHINE_PLATFORM:MNetBSD-9.99.*}",
+ ". if ${MACHINE_PLATFORM:MNetBSD-[1-9][0-9].*}",
+ ". endif",
+ ". endif",
+ ".endif"),
+ "ERROR: filename.mk:7: The patterns \"NetBSD-9.99.*\" from line 6 "+
+ "and \"NetBSD-[1-9][0-9].*\" cannot match at the same time.")
+
+ // TODO: Since MACHINE_PLATFORM always starts with OPSYS, these
+ // conditions contradict each other as well.
+ test(
+ lines(
+ ".if ${OPSYS} == NetBSD",
+ ". if ${MACHINE_PLATFORM:MSunOS-5.*}",
+ ". endif",
+ ".endif"),
+ nil...)
+
+ // Since PKG_OPTIONS is a list and contains several words,
+ // each of them can match one of the patterns.
+ t.SetUpOption("one", "")
+ t.SetUpOption("two", "")
+ test(
+ lines(
+ ".if ${PKG_OPTIONS:Mone} && ${PKG_OPTIONS:Mtwo}",
+ ".endif"),
+ nil...)
+
+ // For variables that are declared individually by a package,
+ // pkglint does not have any type information and thus must
+ // not issue an error here.
+ test(
+ lines(
+ "CUSTOM_VAR=\tone two",
+ ".if ${CUSTOM_VAR:Mone} && ${CUSTOM_VAR:Mtwo}",
+ ".endif"),
+ nil...)
+
+ // In this case, pkglint may infer that CUSTOM_VAR has a
+ // constant value and thus cannot match the second pattern.
+ // As of June 2020, it doesn't do this though.
+ test(
+ lines(
+ "CUSTOM_VAR=\tone",
+ ".if ${CUSTOM_VAR:Mone} && ${CUSTOM_VAR:Mtwo}",
+ ".endif"),
+ nil...)
+
+ // If the variable type is guessed based on the variable name (see
+ // guessVariableType) and is not a list, the error message is correct.
+ test(
+ lines(
+ "CUSTOM_FILE=\tone",
+ ".if ${CUSTOM_FILE:Mone} && ${CUSTOM_FILE:Mtwo}",
+ ".endif"),
+ "NOTE: filename.mk:6: CUSTOM_FILE can be compared using the simpler "+
+ "\"${CUSTOM_FILE} == one\" instead of matching against \":Mone\".",
+ "NOTE: filename.mk:6: CUSTOM_FILE can be compared using the simpler "+
+ "\"${CUSTOM_FILE} == two\" instead of matching against \":Mtwo\".",
+ "ERROR: filename.mk:6: The patterns \"one\" and \"two\" "+
+ "cannot match at the same time.")
+
+ // The conditions from an .if and an .elif don't contradict each other.
+ test(
+ lines(
+ ".if ${OPSYS:MNet*}",
+ ".elif ${OPSYS:MFree*}",
+ ".endif"),
+ nil...)
+
+ // And finally, two conditions that can both match at the same time,
+ // just for the code coverage.
+ // It's strange that the above tests did not include that case.
+ test(
+ lines(
+ ".if ${OPSYS:MNet*} && ${OPSYS:MNetB*}",
+ ".endif"),
+ nil...)
+
+ test(
+ lines(
+ ".if ${OPSYS:M[1} && ${OPSYS:M[2}",
+ ".endif"),
+ "WARN: filename.mk:5: Invalid match pattern \"[1\".",
+ "WARN: filename.mk:5: Invalid match pattern \"[2\".")
+}
+
func (s *Suite) Test_MkCondChecker_checkNotEmpty(c *check.C) {
t := s.Init(c)
@@ -1256,3 +1389,62 @@ func (s *Suite) Test_MkCondChecker_check
test("!${VAR}",
nil...)
}
+
+func (s *Suite) Test_MkCondChecker_checkContradictions(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ test := func(cond string, diagnostics ...string) {
+ mklines := t.NewMkLines("filename.mk",
+ ".if "+cond)
+ mkline := mklines.mklines[0]
+ ck := NewMkCondChecker(mkline, mklines)
+
+ mklines.ForEach(func(mkline *MkLine) { ck.checkContradictions() })
+
+ t.CheckOutput(diagnostics)
+ }
+
+ test("!empty(MACHINE_PLATFORM:Mone) && !empty(MACHINE_PLATFORM:Mtwo)",
+ "ERROR: filename.mk:1: The patterns \"one\" and \"two\" "+
+ "cannot match at the same time.")
+}
+
+func (s *Suite) Test_MkCondChecker_collectFacts(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpVartypes()
+
+ mklines := t.NewMkLines("filename.mk",
+ ".if !empty(OPSYS:Mone) && ${OPSYS:Mtwo}",
+ ". if ${OS_VERSION:Mthree}",
+ ". if ((((${OPSYS:Mfour}))))",
+ ". if (${OPSYS:Mor1} || ${OPSYS:Mor2})", // these are ignored
+ ". if ${OPSYS:Mfive} && (${OPSYS:Msix} && ${OPSYS:Mseven})",
+ ". endif",
+ ". endif",
+ ". endif",
+ ".endif")
+ var facts []VarFact
+
+ mklines.ForEach(func(mkline *MkLine) {
+ ck := NewMkCondChecker(mkline, mklines)
+ if mkline.NeedsCond() {
+ facts = append(facts, ck.collectFacts(mkline)...)
+ }
+ })
+
+ for i, _ := range facts {
+ facts[i].Matches = nil // these would just complicate the comparison
+ }
+ t.CheckDeepEquals(facts, []VarFact{
+ {mklines.mklines[0], "OPSYS", "one", nil},
+ {mklines.mklines[0], "OPSYS", "two", nil},
+ {mklines.mklines[1], "OS_VERSION", "three", nil},
+ {mklines.mklines[2], "OPSYS", "four", nil},
+ {mklines.mklines[4], "OPSYS", "five", nil},
+ {mklines.mklines[4], "OPSYS", "six", nil},
+ {mklines.mklines[4], "OPSYS", "seven", nil},
+ })
+}
Index: pkgsrc/pkgtools/pkglint/files/mkline.go
diff -u pkgsrc/pkgtools/pkglint/files/mkline.go:1.80 pkgsrc/pkgtools/pkglint/files/mkline.go:1.81
--- pkgsrc/pkgtools/pkglint/files/mkline.go:1.80 Mon Jun 1 20:49:54 2020
+++ pkgsrc/pkgtools/pkglint/files/mkline.go Sun Jun 14 11:35:54 2020
@@ -1076,24 +1076,6 @@ type Indentation struct {
levels []indentationLevel
}
-func NewIndentation() *Indentation { return &Indentation{} }
-
-func (ind *Indentation) String() string {
- var s strings.Builder
- for _, level := range ind.levels {
- _, _ = fmt.Fprintf(&s, " %d", level.depth)
- if len(level.conditionalVars) > 0 {
- _, _ = fmt.Fprintf(&s, " (%s)", strings.Join(level.conditionalVars, " "))
- }
- }
- return "[" + trimHspace(s.String()) + "]"
-}
-
-func (ind *Indentation) RememberUsedVariables(cond *MkCond) {
- cond.Walk(&MkCondCallback{
- VarUse: func(varuse *MkVarUse) { ind.AddVar(varuse.varname) }})
-}
-
type indentationLevel struct {
mkline *MkLine // The line in which the indentation started; the .if/.for
depth int // Number of space characters; always a multiple of 2
@@ -1112,6 +1094,24 @@ type indentationLevel struct {
guard bool
}
+func NewIndentation() *Indentation { return &Indentation{} }
+
+func (ind *Indentation) String() string {
+ var s strings.Builder
+ for _, level := range ind.levels {
+ _, _ = fmt.Fprintf(&s, " %d", level.depth)
+ if len(level.conditionalVars) > 0 {
+ _, _ = fmt.Fprintf(&s, " (%s)", strings.Join(level.conditionalVars, " "))
+ }
+ }
+ return "[" + trimHspace(s.String()) + "]"
+}
+
+func (ind *Indentation) RememberUsedVariables(cond *MkCond) {
+ cond.Walk(&MkCondCallback{
+ VarUse: func(varuse *MkVarUse) { ind.AddVar(varuse.varname) }})
+}
+
func (ind *Indentation) IsEmpty() bool {
return len(ind.levels) == 0
}
Index: pkgsrc/pkgtools/pkglint/files/util.go
diff -u pkgsrc/pkgtools/pkglint/files/util.go:1.80 pkgsrc/pkgtools/pkglint/files/util.go:1.81
--- pkgsrc/pkgtools/pkglint/files/util.go:1.80 Fri Jun 12 19:14:45 2020
+++ pkgsrc/pkgtools/pkglint/files/util.go Sun Jun 14 11:35:54 2020
@@ -602,329 +602,6 @@ func (o *Once) check(key uint64) bool {
return true
}
-// Scope remembers which variables are defined and which are used
-// in a certain scope, such as a package or a file.
-//
-// TODO: Decide whether the scope should consider variable assignments
-// from the pkgsrc infrastructure. For Package.checkGnuConfigureUseLanguages
-// it would be better to ignore them completely.
-//
-// TODO: Merge this code with Var, which defines essentially the
-// same features.
-//
-// See also substScope, which already analyzes the possible variable values
-// based on the conditional code paths.
-//
-// See also RedundantScope.
-type Scope struct {
- vs map[string]*scopeVar
- names []string
-}
-
-type scopeVar struct {
- firstDef *MkLine
- lastDef *MkLine
- value string
- used *MkLine
- fallback string
- usedAtLoadTime bool
- indeterminate bool
-}
-
-func NewScope() Scope {
- return Scope{make(map[string]*scopeVar), nil}
-}
-
-func (s *Scope) varnames() []string {
- if len(s.names) == 0 && len(s.vs) > 0 {
- varnames := make([]string, len(s.vs))
- i := 0
- for varname := range s.vs {
- varnames[i] = varname
- i++
- }
- sort.Strings(varnames)
- s.names = varnames
- }
- return s.names
-}
-
-func (s *Scope) create(varname string) *scopeVar {
- if v := s.vs[varname]; v != nil {
- return v
- }
- var sv scopeVar
- s.vs[varname] = &sv
- s.names = nil
- return &sv
-}
-
-// Define marks the variable and its canonicalized form as defined.
-func (s *Scope) Define(varname string, mkline *MkLine) {
- s.def(varname, mkline)
- varcanon := varnameCanon(varname)
- if varcanon != varname {
- s.def(varcanon, mkline)
- }
-}
-
-func (s *Scope) def(name string, mkline *MkLine) {
- v := s.create(name)
- if v.firstDef == nil {
- v.firstDef = mkline
- if trace.Tracing {
- trace.Step2("Defining %q for the first time in %s", name, mkline.String())
- }
- } else if trace.Tracing {
- trace.Step2("Defining %q in %s", name, mkline.String())
- }
-
- v.lastDef = mkline
-
- // In most cases the defining lines are indeed variable assignments.
- // Exceptions are comments from documentation sections, which still mark
- // the variable as defined so that it doesn't produce the "used but not defined" warning;
- // see MkLines.collectDocumentedVariables.
- if !mkline.IsVarassign() {
- return
- }
-
- switch mkline.Op() {
- case opAssignAppend:
- value := mkline.Value()
- if trace.Tracing {
- trace.Stepf("Scope.Define.append %s: %s = %q + %q",
- mkline.String(), name, v.value, value)
- }
- v.value += " " + value
- case opAssignDefault:
- if v.value == "" && !v.indeterminate {
- v.value = mkline.Value()
- }
- case opAssignShell:
- v.value = ""
- v.indeterminate = true
- default:
- v.value = mkline.Value()
- }
-}
-
-func (s *Scope) Fallback(varname string, value string) {
- s.create(varname).fallback = value
-}
-
-// Use marks the variable and its canonicalized form as used.
-func (s *Scope) Use(varname string, line *MkLine, time VucTime) {
- use := func(name string) {
- v := s.create(name)
- if v.used == nil {
- v.used = line
- if trace.Tracing {
- trace.Step2("Using %q in %s", name, line.String())
- }
- }
- if time == VucLoadTime {
- v.usedAtLoadTime = true
- }
- }
-
- use(varname)
- use(varnameCanon(varname))
-}
-
-// Mentioned returns the first line in which the variable is either:
-// - defined,
-// - mentioned in a commented variable assignment,
-// - mentioned in a documentation comment.
-func (s *Scope) Mentioned(varname string) *MkLine {
- if v := s.vs[varname]; v != nil {
- return v.firstDef
- }
- return nil
-}
-
-// IsDefined tests whether the variable is defined.
-// It does NOT test the canonicalized variable name.
-//
-// Even if IsDefined returns true, FirstDefinition doesn't necessarily return true
-// since the latter ignores the default definitions from vardefs.go, keyword dummyVardefMkline.
-func (s *Scope) IsDefined(varname string) bool {
- mkline := s.Mentioned(varname)
- return mkline != nil && mkline.IsVarassign()
-}
-
-// IsDefinedSimilar tests whether the variable or its canonicalized form is defined.
-func (s *Scope) IsDefinedSimilar(varname string) bool {
- if s.IsDefined(varname) {
- if trace.Tracing {
- trace.Step1("Variable %q is defined", varname)
- }
- return true
- }
-
- varcanon := varnameCanon(varname)
- if s.IsDefined(varcanon) {
- if trace.Tracing {
- trace.Step2("Variable %q (similar to %q) is defined", varcanon, varname)
- }
- return true
- }
- return false
-}
-
-// IsUsed tests whether the variable is used.
-// It does NOT test the canonicalized variable name.
-func (s *Scope) IsUsed(varname string) bool {
- return s.FirstUse(varname) != nil
-}
-
-// IsUsedSimilar tests whether the variable or its canonicalized form is used.
-func (s *Scope) IsUsedSimilar(varname string) bool {
- return s.FirstUse(varname) != nil || s.FirstUse(varnameCanon(varname)) != nil
-}
-
-// IsUsedAtLoadTime returns true if the variable is used at load time
-// somewhere.
-func (s *Scope) IsUsedAtLoadTime(varname string) bool {
- v := s.vs[varname]
- return v != nil && v.usedAtLoadTime
-}
-
-// FirstDefinition returns the line in which the variable has been defined first.
-//
-// Having multiple definitions is typical in the branches of "if" statements.
-//
-// Another typical case involves two files: the included file defines a default
-// value, and the including file later overrides that value. Or the other way
-// round: the including file sets a value first, and the included file then
-// assigns a default value using ?=.
-func (s *Scope) FirstDefinition(varname string) *MkLine {
- v := s.vs[varname]
- if v == nil {
- return nil
- }
-
- mkline := v.firstDef
- if mkline != nil && mkline.IsVarassign() {
- lastLine := s.LastDefinition(varname)
- if trace.Tracing && lastLine != mkline {
- trace.Stepf("%s: FirstDefinition differs from LastDefinition in %s.",
- mkline.String(), mkline.RelMkLine(lastLine))
- }
- return mkline
- }
- return nil // See NewPackage and G.Pkgsrc.UserDefinedVars
-}
-
-// LastDefinition returns the line in which the variable has been defined last.
-//
-// Having multiple definitions is typical in the branches of "if" statements.
-//
-// Another typical case involves two files: the included file defines a default
-// value, and the including file later overrides that value. Or the other way
-// round: the including file sets a value first, and the included file then
-// assigns a default value using ?=.
-func (s *Scope) LastDefinition(varname string) *MkLine {
- v := s.vs[varname]
- if v == nil {
- return nil
- }
-
- mkline := v.lastDef
- if mkline != nil && mkline.IsVarassign() {
- return mkline
- }
- return nil // See NewPackage and G.Pkgsrc.UserDefinedVars
-}
-
-// Commented returns whether the variable has only been defined in commented
-// variable assignments. These are ignored by bmake but used heavily in
-// mk/defaults/mk.conf for documentation.
-func (s *Scope) Commented(varname string) *MkLine {
- v := s.vs[varname]
- if v == nil {
- return nil
- }
-
- mklines := make([]*MkLine, 0, 2)
- if v.firstDef != nil {
- mklines = append(mklines, v.firstDef)
- }
- if v.lastDef != nil {
- mklines = append(mklines, v.lastDef)
- }
-
- for _, mkline := range mklines {
- if mkline.IsVarassign() {
- return nil
- }
- }
-
- for _, mkline := range mklines {
- if mkline.IsCommentedVarassign() {
- return mkline
- }
- }
-
- return nil
-}
-
-func (s *Scope) FirstUse(varname string) *MkLine {
- v := s.vs[varname]
- if v == nil {
- return nil
- }
- return v.used
-}
-
-// LastValue returns the value from the last variable definition.
-//
-// If an empty string is returned, this can mean either that the
-// variable value is indeed the empty string or that the variable
-// was not found, or that the variable value cannot be determined
-// reliably. To distinguish these cases, call LastValueFound instead.
-func (s *Scope) LastValue(varname string) string {
- value, _, _ := s.LastValueFound(varname)
- return value
-}
-
-func (s *Scope) LastValueFound(varname string) (value string, found bool, indeterminate bool) {
- v := s.vs[varname]
- if v == nil {
- return
- }
-
- value = v.value
- found = v.firstDef != nil && v.firstDef.IsVarassign()
- indeterminate = v.indeterminate
- if found {
- return
- }
-
- return v.fallback, v.fallback != "", v.indeterminate
-}
-
-func (s *Scope) DefineAll(other *Scope) {
- for _, varname := range other.varnames() {
- v := other.vs[varname]
- if v.firstDef != nil {
- s.Define(varname, v.firstDef)
- s.Define(varname, v.lastDef)
- }
- }
-}
-
-func (s *Scope) forEach(action func(varname string, data *scopeVar)) {
- var keys []string
- for key := range s.vs {
- keys = append(keys, key)
- }
- sort.Strings(keys)
- for _, key := range keys {
- action(key, s.vs[key])
- }
-}
-
// The MIT License (MIT)
//
// Copyright (c) 2015 Frits van Bommel
Index: pkgsrc/pkgtools/pkglint/files/mkparser.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser.go:1.42 pkgsrc/pkgtools/pkglint/files/mkparser.go:1.43
--- pkgsrc/pkgtools/pkglint/files/mkparser.go:1.42 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkparser.go Sun Jun 14 11:35:54 2020
@@ -474,6 +474,7 @@ type MkCondCall struct {
// MkCondCallback defines the actions for walking a Makefile condition
// using MkCondWalker.Walk.
type MkCondCallback struct {
+ And func(conds []*MkCond)
Not func(cond *MkCond)
Defined func(varname string)
Empty func(empty *MkVarUse)
@@ -504,6 +505,9 @@ func (w *MkCondWalker) Walk(cond *MkCond
}
case cond.And != nil:
+ if callback.And != nil {
+ callback.And(cond.And)
+ }
for _, and := range cond.And {
w.Walk(and, callback)
}
Index: pkgsrc/pkgtools/pkglint/files/mkparser_test.go
diff -u pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.39 pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.40
--- pkgsrc/pkgtools/pkglint/files/mkparser_test.go:1.39 Sat Jan 18 21:56:09 2020
+++ pkgsrc/pkgtools/pkglint/files/mkparser_test.go Sun Jun 14 11:35:54 2020
@@ -504,9 +504,13 @@ func (s *Suite) Test_MkCondWalker_Walk(c
events = append(events, sprintf("%14s %s", name, strings.Join(args, ", ")))
}
- // XXX: Add callbacks for And and Or if needed.
+ // XXX: Add callbacks for Or if needed.
+ // A good use case would be to check for unsatisfiable .elif conditions.
mkline.Cond().Walk(&MkCondCallback{
+ func(conds []*MkCond) {
+ addEvent("and")
+ },
func(cond *MkCond) {
addEvent("not")
},
@@ -550,6 +554,7 @@ func (s *Suite) Test_MkCondWalker_Walk(c
" varUse VAR",
" varUse PRE",
" varUse POST",
+ " and ",
" compareVarNum NUM, 3",
" varUse NUM",
" defined VAR",
Index: pkgsrc/pkgtools/pkglint/files/util_test.go
diff -u pkgsrc/pkgtools/pkglint/files/util_test.go:1.54 pkgsrc/pkgtools/pkglint/files/util_test.go:1.55
--- pkgsrc/pkgtools/pkglint/files/util_test.go:1.54 Fri Jun 12 19:14:45 2020
+++ pkgsrc/pkgtools/pkglint/files/util_test.go Sun Jun 14 11:35:54 2020
@@ -523,230 +523,6 @@ func (s *Suite) Test_Once__trace(c *chec
"FirstTime: str, str2")
}
-func (s *Suite) Test_Scope__no_tracing(c *check.C) {
- t := s.Init(c)
-
- scope := NewScope()
- scope.Define("VAR.param", t.NewMkLine("fname.mk", 3, "VAR.param=\tvalue"))
- t.DisableTracing()
-
- t.CheckEquals(scope.IsDefinedSimilar("VAR.param"), true)
- t.CheckEquals(scope.IsDefinedSimilar("VAR.other"), true)
- t.CheckEquals(scope.IsDefinedSimilar("OTHER"), false)
-}
-
-func (s *Suite) Test_Scope__commented_varassign(c *check.C) {
- t := s.Init(c)
-
- mkline := t.NewMkLine("mk/defaults/mk.conf", 3, "#VAR=default")
- scope := NewScope()
- scope.Define("VAR", mkline)
-
- t.CheckEquals(scope.IsDefined("VAR"), false)
- t.Check(scope.FirstDefinition("VAR"), check.IsNil)
- t.Check(scope.LastDefinition("VAR"), check.IsNil)
-
- t.CheckEquals(scope.Mentioned("VAR"), mkline)
- t.CheckEquals(scope.Commented("VAR"), mkline)
-
- value, found, indeterminate := scope.LastValueFound("VAR")
- t.CheckEquals(value, "")
- t.CheckEquals(found, false)
- t.CheckEquals(indeterminate, false)
-}
-
-func (s *Suite) Test_Scope_Define(c *check.C) {
- t := s.Init(c)
-
- scope := NewScope()
-
- test := func(line string, found, indeterminate bool, value string) {
- mkline := t.NewMkLine("file.mk", 123, line)
- scope.Define("BUILD_DIRS", mkline)
-
- actualValue, actualFound, actualIndeterminate := scope.LastValueFound("BUILD_DIRS")
-
- t.CheckDeepEquals(
- []interface{}{actualFound, actualIndeterminate, actualValue},
- []interface{}{found, indeterminate, value})
- t.CheckEquals(scope.vs["BUILD_DIRS"].value, value)
- }
-
- test("BUILD_DIRS?=\tdefault",
- true, false, "default")
-
- test(
- "BUILD_DIRS=\tone two three",
- true, false, "one two three")
-
- test(
- "BUILD_DIRS+=\tfour",
- true, false, "one two three four")
-
- // Later default assignments do not have an effect.
- test("BUILD_DIRS?=\tdefault",
- true, false, "one two three four")
-
- test("BUILD_DIRS!=\techo dynamic",
- true, true, "")
-
- // The shell assignment above sets the variable to an indeterminate
- // value, after which all further default assignments are ignored.
- test("BUILD_DIRS?=\tdefault after shell assignment",
- true, true, "")
-}
-
-func (s *Suite) Test_Scope_Mentioned(c *check.C) {
- t := s.Init(c)
-
- assigned := t.NewMkLine("filename.mk", 3, "VAR=\tvalue")
- commented := t.NewMkLine("filename.mk", 4, "#COMMENTED=\tvalue")
- documented := t.NewMkLine("filename.mk", 5, "# DOCUMENTED is a variable.")
-
- scope := NewScope()
- scope.Define("VAR", assigned)
- scope.Define("COMMENTED", commented)
- scope.Define("DOCUMENTED", documented)
-
- t.CheckEquals(scope.Mentioned("VAR"), assigned)
- t.CheckEquals(scope.Mentioned("COMMENTED"), commented)
- t.CheckEquals(scope.Mentioned("DOCUMENTED"), documented)
- t.Check(scope.Mentioned("UNKNOWN"), check.IsNil)
-}
-
-func (s *Suite) Test_Scope_IsDefined(c *check.C) {
- t := s.Init(c)
-
- scope := NewScope()
- scope.Define("VAR.param", t.NewMkLine("file.mk", 1, "VAR.param=value"))
-
- t.CheckEquals(scope.IsDefined("VAR.param"), true)
- t.CheckEquals(scope.IsDefined("VAR.other"), false)
- t.CheckEquals(scope.IsDefined("VARIABLE.*"), false)
-
- t.CheckEquals(scope.IsDefinedSimilar("VAR.param"), true)
- t.CheckEquals(scope.IsDefinedSimilar("VAR.other"), true)
- t.CheckEquals(scope.IsDefinedSimilar("VARIABLE.*"), false)
-}
-
-func (s *Suite) Test_Scope_IsUsed(c *check.C) {
- t := s.Init(c)
-
- scope := NewScope()
- mkline := t.NewMkLine("file.mk", 1, "\techo ${VAR.param}")
- scope.Use("VAR.param", mkline, VucRunTime)
-
- t.CheckEquals(scope.IsUsed("VAR.param"), true)
- t.CheckEquals(scope.IsUsed("VAR.other"), false)
- t.CheckEquals(scope.IsUsed("VARIABLE.*"), false)
-
- t.CheckEquals(scope.IsUsedSimilar("VAR.param"), true)
- t.CheckEquals(scope.IsUsedSimilar("VAR.other"), true)
- t.CheckEquals(scope.IsUsedSimilar("VARIABLE.*"), false)
-}
-
-func (s *Suite) Test_Scope_FirstDefinition(c *check.C) {
- t := s.Init(c)
-
- mkline1 := t.NewMkLine("fname.mk", 3, "VAR=\tvalue")
- mkline2 := t.NewMkLine("fname.mk", 3, ".if ${SNEAKY::=value}")
-
- scope := NewScope()
- scope.Define("VAR", mkline1)
- scope.Define("SNEAKY", mkline2)
-
- t.CheckEquals(scope.FirstDefinition("VAR"), mkline1)
-
- // This call returns nil because it's not a variable assignment
- // and the calling code typically assumes a variable definition.
- // These sneaky variables with implicit definition are an edge
- // case that only few people actually know. It's better that way.
- t.Check(scope.FirstDefinition("SNEAKY"), check.IsNil)
-
- t.CheckOutputLines(
- "ERROR: fname.mk:3: Assignment modifiers like \":=\" " +
- "must not be used at all.")
-}
-
-func (s *Suite) Test_Scope_Commented(c *check.C) {
- t := s.Init(c)
-
- assigned := t.NewMkLine("filename.mk", 3, "VAR=\tvalue")
- commented := t.NewMkLine("filename.mk", 4, "#COMMENTED=\tvalue")
- documented := t.NewMkLine("filename.mk", 5, "# DOCUMENTED is a variable.")
-
- scope := NewScope()
- scope.Define("VAR", assigned)
- scope.Define("COMMENTED", commented)
- scope.Define("DOCUMENTED", documented)
-
- t.Check(scope.Commented("VAR"), check.IsNil)
- t.CheckEquals(scope.Commented("COMMENTED"), commented)
- t.Check(scope.Commented("DOCUMENTED"), check.IsNil)
- t.Check(scope.Commented("UNKNOWN"), check.IsNil)
-}
-
-func (s *Suite) Test_Scope_LastValue(c *check.C) {
- t := s.Init(c)
-
- mklines := t.NewMkLines("file.mk",
- MkCvsID,
- "VAR=\tfirst",
- "VAR=\tsecond",
- ".if 1",
- "VAR=\tthird (conditional)",
- ".endif")
-
- mklines.Check()
-
- // TODO: At load time, use loadVars instead of allVars.
- t.CheckEquals(mklines.allVars.LastValue("VAR"), "third (conditional)")
-
- t.CheckOutputLines(
- "WARN: file.mk:2: VAR is defined but not used.")
-}
-
-// Up to 2020-01-06, pkglint wrongly returned "one" as the variable value,
-// even though Makefile.common is included before appending "two".
-func (s *Suite) Test_Scope_LastValue__append_in_multiple_files(c *check.C) {
- t := s.Init(c)
-
- t.SetUpPackage("category/package",
- ".include \"Makefile.common\"",
- "PLIST_VARS+=\ttwo",
- "PLIST.two=\tyes")
- t.Chdir("category/package")
- t.CreateFileLines("PLIST",
- PlistCvsID,
- "${PLIST.one}${PLIST.two}bin/program")
- t.CreateFileLines("Makefile.common",
- MkCvsID,
- "PLIST_VARS=\tone",
- "PLIST.one=\tyes")
- pkg := NewPackage(".")
- t.FinishSetUp()
-
- pkg.Check()
-
- t.CheckEquals(pkg.vars.LastValue("PLIST_VARS"), "one two")
-}
-
-func (s *Suite) Test_Scope_DefineAll(c *check.C) {
- t := s.Init(c)
-
- src := NewScope()
-
- dst := NewScope()
- dst.DefineAll(&src)
-
- c.Check(dst.vs, check.HasLen, 0)
-
- src.Define("VAR", t.NewMkLine("file.mk", 1, "VAR=value"))
- dst.DefineAll(&src)
-
- t.CheckEquals(dst.IsDefined("VAR"), true)
-}
-
func (s *Suite) Test_naturalLess(c *check.C) {
t := s.Init(c)
Added files:
Index: pkgsrc/pkgtools/pkglint/files/scope.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/scope.go:1.1
--- /dev/null Sun Jun 14 11:35:54 2020
+++ pkgsrc/pkgtools/pkglint/files/scope.go Sun Jun 14 11:35:54 2020
@@ -0,0 +1,326 @@
+package pkglint
+
+import "sort"
+
+// Scope remembers which variables are defined and which are used
+// in a certain scope, such as a package or a file.
+//
+// TODO: Decide whether the scope should consider variable assignments
+// from the pkgsrc infrastructure. For Package.checkGnuConfigureUseLanguages
+// it would be better to ignore them completely.
+//
+// TODO: Merge this code with Var, which defines essentially the
+// same features.
+//
+// See also substScope, which already analyzes the possible variable values
+// based on the conditional code paths.
+//
+// See also RedundantScope.
+type Scope struct {
+ vs map[string]*scopeVar
+ names []string
+}
+
+type scopeVar struct {
+ firstDef *MkLine
+ lastDef *MkLine
+ value string
+ used *MkLine
+ fallback string
+ usedAtLoadTime bool
+ indeterminate bool
+}
+
+func NewScope() Scope {
+ return Scope{make(map[string]*scopeVar), nil}
+}
+
+func (s *Scope) varnames() []string {
+ if len(s.names) == 0 && len(s.vs) > 0 {
+ varnames := make([]string, len(s.vs))
+ i := 0
+ for varname := range s.vs {
+ varnames[i] = varname
+ i++
+ }
+ sort.Strings(varnames)
+ s.names = varnames
+ }
+ return s.names
+}
+
+func (s *Scope) create(varname string) *scopeVar {
+ if v := s.vs[varname]; v != nil {
+ return v
+ }
+ var sv scopeVar
+ s.vs[varname] = &sv
+ s.names = nil
+ return &sv
+}
+
+// Define marks the variable and its canonicalized form as defined.
+func (s *Scope) Define(varname string, mkline *MkLine) {
+ s.def(varname, mkline)
+ varcanon := varnameCanon(varname)
+ if varcanon != varname {
+ s.def(varcanon, mkline)
+ }
+}
+
+func (s *Scope) def(name string, mkline *MkLine) {
+ v := s.create(name)
+ if v.firstDef == nil {
+ v.firstDef = mkline
+ if trace.Tracing {
+ trace.Step2("Defining %q for the first time in %s", name, mkline.String())
+ }
+ } else if trace.Tracing {
+ trace.Step2("Defining %q in %s", name, mkline.String())
+ }
+
+ v.lastDef = mkline
+
+ // In most cases the defining lines are indeed variable assignments.
+ // Exceptions are comments from documentation sections, which still mark
+ // the variable as defined so that it doesn't produce the "used but not defined" warning;
+ // see MkLines.collectDocumentedVariables.
+ if !mkline.IsVarassign() {
+ return
+ }
+
+ switch mkline.Op() {
+ case opAssignAppend:
+ value := mkline.Value()
+ if trace.Tracing {
+ trace.Stepf("Scope.Define.append %s: %s = %q + %q",
+ mkline.String(), name, v.value, value)
+ }
+ v.value += " " + value
+ case opAssignDefault:
+ if v.value == "" && !v.indeterminate {
+ v.value = mkline.Value()
+ }
+ case opAssignShell:
+ v.value = ""
+ v.indeterminate = true
+ default:
+ v.value = mkline.Value()
+ }
+}
+
+func (s *Scope) Fallback(varname string, value string) {
+ s.create(varname).fallback = value
+}
+
+// Use marks the variable and its canonicalized form as used.
+func (s *Scope) Use(varname string, line *MkLine, time VucTime) {
+ use := func(name string) {
+ v := s.create(name)
+ if v.used == nil {
+ v.used = line
+ if trace.Tracing {
+ trace.Step2("Using %q in %s", name, line.String())
+ }
+ }
+ if time == VucLoadTime {
+ v.usedAtLoadTime = true
+ }
+ }
+
+ use(varname)
+ use(varnameCanon(varname))
+}
+
+// Mentioned returns the first line in which the variable is either:
+// - defined,
+// - mentioned in a commented variable assignment,
+// - mentioned in a documentation comment.
+func (s *Scope) Mentioned(varname string) *MkLine {
+ if v := s.vs[varname]; v != nil {
+ return v.firstDef
+ }
+ return nil
+}
+
+// IsDefined tests whether the variable is defined.
+// It does NOT test the canonicalized variable name.
+//
+// Even if IsDefined returns true, FirstDefinition doesn't necessarily return true
+// since the latter ignores the default definitions from vardefs.go, keyword dummyVardefMkline.
+func (s *Scope) IsDefined(varname string) bool {
+ mkline := s.Mentioned(varname)
+ return mkline != nil && mkline.IsVarassign()
+}
+
+// IsDefinedSimilar tests whether the variable or its canonicalized form is defined.
+func (s *Scope) IsDefinedSimilar(varname string) bool {
+ if s.IsDefined(varname) {
+ if trace.Tracing {
+ trace.Step1("Variable %q is defined", varname)
+ }
+ return true
+ }
+
+ varcanon := varnameCanon(varname)
+ if s.IsDefined(varcanon) {
+ if trace.Tracing {
+ trace.Step2("Variable %q (similar to %q) is defined", varcanon, varname)
+ }
+ return true
+ }
+ return false
+}
+
+// IsUsed tests whether the variable is used.
+// It does NOT test the canonicalized variable name.
+func (s *Scope) IsUsed(varname string) bool {
+ return s.FirstUse(varname) != nil
+}
+
+// IsUsedSimilar tests whether the variable or its canonicalized form is used.
+func (s *Scope) IsUsedSimilar(varname string) bool {
+ return s.FirstUse(varname) != nil || s.FirstUse(varnameCanon(varname)) != nil
+}
+
+// IsUsedAtLoadTime returns true if the variable is used at load time
+// somewhere.
+func (s *Scope) IsUsedAtLoadTime(varname string) bool {
+ v := s.vs[varname]
+ return v != nil && v.usedAtLoadTime
+}
+
+// FirstDefinition returns the line in which the variable has been defined first.
+//
+// Having multiple definitions is typical in the branches of "if" statements.
+//
+// Another typical case involves two files: the included file defines a default
+// value, and the including file later overrides that value. Or the other way
+// round: the including file sets a value first, and the included file then
+// assigns a default value using ?=.
+func (s *Scope) FirstDefinition(varname string) *MkLine {
+ v := s.vs[varname]
+ if v == nil {
+ return nil
+ }
+
+ mkline := v.firstDef
+ if mkline != nil && mkline.IsVarassign() {
+ lastLine := s.LastDefinition(varname)
+ if trace.Tracing && lastLine != mkline {
+ trace.Stepf("%s: FirstDefinition differs from LastDefinition in %s.",
+ mkline.String(), mkline.RelMkLine(lastLine))
+ }
+ return mkline
+ }
+ return nil // See NewPackage and G.Pkgsrc.UserDefinedVars
+}
+
+// LastDefinition returns the line in which the variable has been defined last.
+//
+// Having multiple definitions is typical in the branches of "if" statements.
+//
+// Another typical case involves two files: the included file defines a default
+// value, and the including file later overrides that value. Or the other way
+// round: the including file sets a value first, and the included file then
+// assigns a default value using ?=.
+func (s *Scope) LastDefinition(varname string) *MkLine {
+ v := s.vs[varname]
+ if v == nil {
+ return nil
+ }
+
+ mkline := v.lastDef
+ if mkline != nil && mkline.IsVarassign() {
+ return mkline
+ }
+ return nil // See NewPackage and G.Pkgsrc.UserDefinedVars
+}
+
+// Commented returns whether the variable has only been defined in commented
+// variable assignments. These are ignored by bmake but used heavily in
+// mk/defaults/mk.conf for documentation.
+func (s *Scope) Commented(varname string) *MkLine {
+ v := s.vs[varname]
+ if v == nil {
+ return nil
+ }
+
+ mklines := make([]*MkLine, 0, 2)
+ if v.firstDef != nil {
+ mklines = append(mklines, v.firstDef)
+ }
+ if v.lastDef != nil {
+ mklines = append(mklines, v.lastDef)
+ }
+
+ for _, mkline := range mklines {
+ if mkline.IsVarassign() {
+ return nil
+ }
+ }
+
+ for _, mkline := range mklines {
+ if mkline.IsCommentedVarassign() {
+ return mkline
+ }
+ }
+
+ return nil
+}
+
+func (s *Scope) FirstUse(varname string) *MkLine {
+ v := s.vs[varname]
+ if v == nil {
+ return nil
+ }
+ return v.used
+}
+
+// LastValue returns the value from the last variable definition.
+//
+// If an empty string is returned, this can mean either that the
+// variable value is indeed the empty string or that the variable
+// was not found, or that the variable value cannot be determined
+// reliably. To distinguish these cases, call LastValueFound instead.
+func (s *Scope) LastValue(varname string) string {
+ value, _, _ := s.LastValueFound(varname)
+ return value
+}
+
+func (s *Scope) LastValueFound(varname string) (value string, found bool, indeterminate bool) {
+ v := s.vs[varname]
+ if v == nil {
+ return
+ }
+
+ value = v.value
+ found = v.firstDef != nil && v.firstDef.IsVarassign()
+ indeterminate = v.indeterminate
+ if found {
+ return
+ }
+
+ return v.fallback, v.fallback != "", v.indeterminate
+}
+
+func (s *Scope) DefineAll(other *Scope) {
+ for _, varname := range other.varnames() {
+ v := other.vs[varname]
+ if v.firstDef != nil {
+ s.Define(varname, v.firstDef)
+ s.Define(varname, v.lastDef)
+ }
+ }
+}
+
+func (s *Scope) forEach(action func(varname string, data *scopeVar)) {
+ var keys []string
+ for key := range s.vs {
+ keys = append(keys, key)
+ }
+ sort.Strings(keys)
+ for _, key := range keys {
+ action(key, s.vs[key])
+ }
+}
Index: pkgsrc/pkgtools/pkglint/files/scope_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/scope_test.go:1.1
--- /dev/null Sun Jun 14 11:35:54 2020
+++ pkgsrc/pkgtools/pkglint/files/scope_test.go Sun Jun 14 11:35:54 2020
@@ -0,0 +1,227 @@
+package pkglint
+
+import "gopkg.in/check.v1"
+
+func (s *Suite) Test_Scope__no_tracing(c *check.C) {
+ t := s.Init(c)
+
+ scope := NewScope()
+ scope.Define("VAR.param", t.NewMkLine("fname.mk", 3, "VAR.param=\tvalue"))
+ t.DisableTracing()
+
+ t.CheckEquals(scope.IsDefinedSimilar("VAR.param"), true)
+ t.CheckEquals(scope.IsDefinedSimilar("VAR.other"), true)
+ t.CheckEquals(scope.IsDefinedSimilar("OTHER"), false)
+}
+
+func (s *Suite) Test_Scope__commented_varassign(c *check.C) {
+ t := s.Init(c)
+
+ mkline := t.NewMkLine("mk/defaults/mk.conf", 3, "#VAR=default")
+ scope := NewScope()
+ scope.Define("VAR", mkline)
+
+ t.CheckEquals(scope.IsDefined("VAR"), false)
+ t.Check(scope.FirstDefinition("VAR"), check.IsNil)
+ t.Check(scope.LastDefinition("VAR"), check.IsNil)
+
+ t.CheckEquals(scope.Mentioned("VAR"), mkline)
+ t.CheckEquals(scope.Commented("VAR"), mkline)
+
+ value, found, indeterminate := scope.LastValueFound("VAR")
+ t.CheckEquals(value, "")
+ t.CheckEquals(found, false)
+ t.CheckEquals(indeterminate, false)
+}
+
+func (s *Suite) Test_Scope_Define(c *check.C) {
+ t := s.Init(c)
+
+ scope := NewScope()
+
+ test := func(line string, found, indeterminate bool, value string) {
+ mkline := t.NewMkLine("file.mk", 123, line)
+ scope.Define("BUILD_DIRS", mkline)
+
+ actualValue, actualFound, actualIndeterminate := scope.LastValueFound("BUILD_DIRS")
+
+ t.CheckDeepEquals(
+ []interface{}{actualFound, actualIndeterminate, actualValue},
+ []interface{}{found, indeterminate, value})
+ t.CheckEquals(scope.vs["BUILD_DIRS"].value, value)
+ }
+
+ test("BUILD_DIRS?=\tdefault",
+ true, false, "default")
+
+ test(
+ "BUILD_DIRS=\tone two three",
+ true, false, "one two three")
+
+ test(
+ "BUILD_DIRS+=\tfour",
+ true, false, "one two three four")
+
+ // Later default assignments do not have an effect.
+ test("BUILD_DIRS?=\tdefault",
+ true, false, "one two three four")
+
+ test("BUILD_DIRS!=\techo dynamic",
+ true, true, "")
+
+ // The shell assignment above sets the variable to an indeterminate
+ // value, after which all further default assignments are ignored.
+ test("BUILD_DIRS?=\tdefault after shell assignment",
+ true, true, "")
+}
+
+func (s *Suite) Test_Scope_Mentioned(c *check.C) {
+ t := s.Init(c)
+
+ assigned := t.NewMkLine("filename.mk", 3, "VAR=\tvalue")
+ commented := t.NewMkLine("filename.mk", 4, "#COMMENTED=\tvalue")
+ documented := t.NewMkLine("filename.mk", 5, "# DOCUMENTED is a variable.")
+
+ scope := NewScope()
+ scope.Define("VAR", assigned)
+ scope.Define("COMMENTED", commented)
+ scope.Define("DOCUMENTED", documented)
+
+ t.CheckEquals(scope.Mentioned("VAR"), assigned)
+ t.CheckEquals(scope.Mentioned("COMMENTED"), commented)
+ t.CheckEquals(scope.Mentioned("DOCUMENTED"), documented)
+ t.Check(scope.Mentioned("UNKNOWN"), check.IsNil)
+}
+
+func (s *Suite) Test_Scope_IsDefined(c *check.C) {
+ t := s.Init(c)
+
+ scope := NewScope()
+ scope.Define("VAR.param", t.NewMkLine("file.mk", 1, "VAR.param=value"))
+
+ t.CheckEquals(scope.IsDefined("VAR.param"), true)
+ t.CheckEquals(scope.IsDefined("VAR.other"), false)
+ t.CheckEquals(scope.IsDefined("VARIABLE.*"), false)
+
+ t.CheckEquals(scope.IsDefinedSimilar("VAR.param"), true)
+ t.CheckEquals(scope.IsDefinedSimilar("VAR.other"), true)
+ t.CheckEquals(scope.IsDefinedSimilar("VARIABLE.*"), false)
+}
+
+func (s *Suite) Test_Scope_IsUsed(c *check.C) {
+ t := s.Init(c)
+
+ scope := NewScope()
+ mkline := t.NewMkLine("file.mk", 1, "\techo ${VAR.param}")
+ scope.Use("VAR.param", mkline, VucRunTime)
+
+ t.CheckEquals(scope.IsUsed("VAR.param"), true)
+ t.CheckEquals(scope.IsUsed("VAR.other"), false)
+ t.CheckEquals(scope.IsUsed("VARIABLE.*"), false)
+
+ t.CheckEquals(scope.IsUsedSimilar("VAR.param"), true)
+ t.CheckEquals(scope.IsUsedSimilar("VAR.other"), true)
+ t.CheckEquals(scope.IsUsedSimilar("VARIABLE.*"), false)
+}
+
+func (s *Suite) Test_Scope_FirstDefinition(c *check.C) {
+ t := s.Init(c)
+
+ mkline1 := t.NewMkLine("fname.mk", 3, "VAR=\tvalue")
+ mkline2 := t.NewMkLine("fname.mk", 3, ".if ${SNEAKY::=value}")
+
+ scope := NewScope()
+ scope.Define("VAR", mkline1)
+ scope.Define("SNEAKY", mkline2)
+
+ t.CheckEquals(scope.FirstDefinition("VAR"), mkline1)
+
+ // This call returns nil because it's not a variable assignment
+ // and the calling code typically assumes a variable definition.
+ // These sneaky variables with implicit definition are an edge
+ // case that only few people actually know. It's better that way.
+ t.Check(scope.FirstDefinition("SNEAKY"), check.IsNil)
+
+ t.CheckOutputLines(
+ "ERROR: fname.mk:3: Assignment modifiers like \":=\" " +
+ "must not be used at all.")
+}
+
+func (s *Suite) Test_Scope_Commented(c *check.C) {
+ t := s.Init(c)
+
+ assigned := t.NewMkLine("filename.mk", 3, "VAR=\tvalue")
+ commented := t.NewMkLine("filename.mk", 4, "#COMMENTED=\tvalue")
+ documented := t.NewMkLine("filename.mk", 5, "# DOCUMENTED is a variable.")
+
+ scope := NewScope()
+ scope.Define("VAR", assigned)
+ scope.Define("COMMENTED", commented)
+ scope.Define("DOCUMENTED", documented)
+
+ t.Check(scope.Commented("VAR"), check.IsNil)
+ t.CheckEquals(scope.Commented("COMMENTED"), commented)
+ t.Check(scope.Commented("DOCUMENTED"), check.IsNil)
+ t.Check(scope.Commented("UNKNOWN"), check.IsNil)
+}
+
+func (s *Suite) Test_Scope_LastValue(c *check.C) {
+ t := s.Init(c)
+
+ mklines := t.NewMkLines("file.mk",
+ MkCvsID,
+ "VAR=\tfirst",
+ "VAR=\tsecond",
+ ".if 1",
+ "VAR=\tthird (conditional)",
+ ".endif")
+
+ mklines.Check()
+
+ // TODO: At load time, use loadVars instead of allVars.
+ t.CheckEquals(mklines.allVars.LastValue("VAR"), "third (conditional)")
+
+ t.CheckOutputLines(
+ "WARN: file.mk:2: VAR is defined but not used.")
+}
+
+// Up to 2020-01-06, pkglint wrongly returned "one" as the variable value,
+// even though Makefile.common is included before appending "two".
+func (s *Suite) Test_Scope_LastValue__append_in_multiple_files(c *check.C) {
+ t := s.Init(c)
+
+ t.SetUpPackage("category/package",
+ ".include \"Makefile.common\"",
+ "PLIST_VARS+=\ttwo",
+ "PLIST.two=\tyes")
+ t.Chdir("category/package")
+ t.CreateFileLines("PLIST",
+ PlistCvsID,
+ "${PLIST.one}${PLIST.two}bin/program")
+ t.CreateFileLines("Makefile.common",
+ MkCvsID,
+ "PLIST_VARS=\tone",
+ "PLIST.one=\tyes")
+ pkg := NewPackage(".")
+ t.FinishSetUp()
+
+ pkg.Check()
+
+ t.CheckEquals(pkg.vars.LastValue("PLIST_VARS"), "one two")
+}
+
+func (s *Suite) Test_Scope_DefineAll(c *check.C) {
+ t := s.Init(c)
+
+ src := NewScope()
+
+ dst := NewScope()
+ dst.DefineAll(&src)
+
+ c.Check(dst.vs, check.HasLen, 0)
+
+ src.Define("VAR", t.NewMkLine("file.mk", 1, "VAR=value"))
+ dst.DefineAll(&src)
+
+ t.CheckEquals(dst.IsDefined("VAR"), true)
+}
Index: pkgsrc/pkgtools/pkglint/files/makepat/pat.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/makepat/pat.go:1.1
--- /dev/null Sun Jun 14 11:35:54 2020
+++ pkgsrc/pkgtools/pkglint/files/makepat/pat.go Sun Jun 14 11:35:54 2020
@@ -0,0 +1,208 @@
+package makepat
+
+import (
+ "errors"
+ "netbsd.org/pkglint/textproc"
+)
+
+// Pattern is a compiled pattern like "*.c" or "NetBSD-??.[^0-9]".
+// It behaves exactly like in bmake,
+// see devel/bmake/files/str.c, function Str_Match.
+type Pattern struct {
+ states []state
+}
+
+type state struct {
+ transitions []transition
+ end bool
+}
+
+type transition struct {
+ min, max byte
+ to StateID
+}
+
+type StateID uint16
+
+// Compile parses a pattern, including the error checking that is missing
+// from bmake.
+func Compile(pattern string) (*Pattern, error) {
+ var a Pattern
+ s := a.AddState(false)
+
+ var deadEnd StateID
+
+ lex := textproc.NewLexer(pattern)
+ for !lex.EOF() {
+
+ if lex.SkipByte('*') {
+ a.AddTransition(s, 0, 255, s)
+ continue
+ }
+
+ if lex.SkipByte('?') {
+ next := a.AddState(false)
+ a.AddTransition(s, 0, 255, next)
+ s = next
+ continue
+ }
+
+ if lex.SkipByte('\\') {
+ if lex.EOF() {
+ return nil, errors.New("unfinished escape sequence")
+ }
+ ch := lex.NextByte()
+ next := a.AddState(false)
+ a.AddTransition(s, ch, ch, next)
+ s = next
+ continue
+ }
+
+ ch := lex.NextByte()
+ if ch != '[' {
+ next := a.AddState(false)
+ a.AddTransition(s, ch, ch, next)
+ s = next
+ continue
+ }
+
+ negate := lex.SkipByte('^')
+ if negate && deadEnd == 0 {
+ deadEnd = a.AddState(false)
+ }
+ next := a.AddState(false)
+ for {
+ if lex.EOF() {
+ return nil, errors.New("unfinished character class")
+ }
+ ch = lex.NextByte()
+ if ch == ']' {
+ break
+ }
+ max := ch
+ if lex.SkipByte('-') {
+ if lex.EOF() {
+ return nil, errors.New("unfinished character range")
+ }
+ max = lex.NextByte()
+ }
+
+ to := next
+ if negate {
+ to = deadEnd
+ }
+ a.AddTransition(s, bmin(ch, max), bmax(ch, max), to)
+ }
+ if negate {
+ a.AddTransition(s, 0, 255, next)
+ }
+ s = next
+ }
+
+ a.states[s].end = true
+ return &a, nil
+}
+
+func (a *Pattern) AddState(end bool) StateID {
+ a.states = append(a.states, state{nil, end})
+ return StateID(len(a.states) - 1)
+}
+
+func (a *Pattern) AddTransition(from StateID, min, max byte, to StateID) {
+ state := &a.states[from]
+ state.transitions = append(state.transitions, transition{min, max, to})
+}
+
+// Match tests whether a pattern matches the given string.
+func (a *Pattern) Match(s string) bool {
+ state := StateID(0)
+ for _, ch := range []byte(s) {
+ for _, tr := range a.states[state].transitions {
+ if tr.min <= ch && ch <= tr.max {
+ state = tr.to
+ goto nextByte
+ }
+ }
+ return false
+ nextByte:
+ }
+ return a.states[state].end
+}
+
+// Intersect computes a pattern that only matches if both given patterns
+// match at the same time.
+func Intersect(a, b *Pattern) *Pattern {
+ var is Pattern
+ for i := 0; i < len(a.states); i++ {
+ for j := 0; j < len(b.states); j++ {
+ is.AddState(a.states[i].end && b.states[j].end)
+ }
+ }
+
+ for i := 0; i < len(a.states); i++ {
+ for j := 0; j < len(b.states); j++ {
+ for _, at := range a.states[i].transitions {
+ for _, bt := range b.states[j].transitions {
+ min := bmax(at.min, bt.min)
+ max := bmin(at.max, bt.max)
+ if min <= max {
+ from := StateID(i*len(b.states) + j)
+ to := at.to*StateID(len(b.states)) + bt.to
+ is.AddTransition(from, min, max, to)
+ }
+ }
+ }
+ }
+ }
+
+ // TODO: optimize: remove transitions that point to a dead end
+
+ return &is
+}
+
+// CanMatch tests whether the pattern can match some string.
+// Most patterns can do that.
+// Typical counterexamples are:
+// [^]
+// Intersect("*.c", "*.h")
+func (a *Pattern) CanMatch() bool {
+ reachable := make([]bool, len(a.states))
+ reachable[0] = true
+
+again:
+ changed := false
+ for i, s := range a.states {
+ if reachable[i] {
+ for _, t := range s.transitions {
+ if !reachable[t.to] {
+ reachable[t.to] = true
+ changed = true
+ }
+ }
+ }
+ }
+ if changed {
+ goto again
+ }
+
+ for i, s := range a.states {
+ if reachable[i] && s.end {
+ return true
+ }
+ }
+ return false
+}
+
+func bmin(a, b byte) byte {
+ if a < b {
+ return a
+ }
+ return b
+}
+
+func bmax(a, b byte) byte {
+ if a > b {
+ return a
+ }
+ return b
+}
Index: pkgsrc/pkgtools/pkglint/files/makepat/pat_test.go
diff -u /dev/null pkgsrc/pkgtools/pkglint/files/makepat/pat_test.go:1.1
--- /dev/null Sun Jun 14 11:35:54 2020
+++ pkgsrc/pkgtools/pkglint/files/makepat/pat_test.go Sun Jun 14 11:35:54 2020
@@ -0,0 +1,139 @@
+package makepat
+
+import (
+ "testing"
+)
+
+func Test_Automaton_Match(t *testing.T) {
+ tests := []struct {
+ pattern string
+ str string
+ want bool
+ }{
+ {"a-[0-9]*", "", false},
+ {"a-[0-9]*", "a", false},
+ {"a-[0-9]*", "b", false},
+ {"a-[0-9]*", "a-", false},
+ {"a-[0-9]*", "a-0", true},
+ {"a-[0-9]*", "a-13", true},
+ {"a-[0-9]*", "a-3* matches arbitrary text", true},
+ {"a-[0-9]*", "a+", false},
+ {"a-[0-9]*", "a-x", false},
+ {"\\a", "a", true},
+ {"\\a", "\\a", false},
+ {"\\a", "\u0007", false},
+ {"?", "", false},
+ {"?", "x", true},
+ {"?", "?", true},
+ {"?", "xy", false},
+ {"a?", "ax", true},
+ {"a?", "xa", false},
+ {"[0-9]", "", false},
+ {"[0-9]", "55", false},
+ {"[0-9]", "/", false},
+ {"[0-9]", "0", true},
+ {"[0-9]", "5", true},
+ {"[0-9]", "9", true},
+ {"[0-9]", ":", false},
+ {"[^0-9]", "", false},
+ {"[^0-9]", "55", false},
+ {"[^0-9]", "/", true},
+ {"[^0-9]", "0", false},
+ {"[^0-9]", "5", false},
+ {"[^0-9]", "9", false},
+ {"[^0-9]", ":", true},
+ {"[^0-9][^a-z]", "a0", true},
+ {"[^0-9][^a-z]", "aa", false},
+ {"[0-9A-Za-z]", "/", false},
+ {"[0-9A-Za-z]", "0", true},
+ {"[0-9A-Za-z]", "9", true},
+ {"[0-9A-Za-z]", ":", false},
+ {"[0-9A-Za-z]", "@", false},
+ {"[0-9A-Za-z]", "A", true},
+ {"[0-9A-Za-z]", "Z", true},
+ {"[0-9A-Za-z]", "[", false},
+ {"[0-9A-Za-z]", "`", false},
+ {"[0-9A-Za-z]", "a", true},
+ {"[0-9A-Za-z]", "z", true},
+ {"[0-9A-Za-z]", "{", false},
+ {"[\\-]]", "{", false},
+ {"[\\-]]", "\\", true},
+ {"[\\-]]", "]", true},
+ {"[\\-]]", "^", false},
+ {"[9-0]", "", false},
+ {"[9-0]", "55", false},
+ {"[9-0]", "/", false},
+ {"[9-0]", "0", true},
+ {"[9-0]", "5", true},
+ {"[9-0]", "9", true},
+ {"[9-0]", ":", false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.pattern+" "+tt.str, func(t *testing.T) {
+ a, err := Compile(tt.pattern)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if got := a.Match(tt.str); got != tt.want {
+ t.Errorf("Match() = %v, want %v", got, tt.want)
+ }
+ })
+ }
+}
+
+func Test_Automaton_Compile__errors(t *testing.T) {
+ tests := []struct {
+ pattern string
+ msg string
+ }{
+ {"\\", "unfinished escape sequence"},
+ {"[", "unfinished character class"},
+ {"[a-", "unfinished character range"},
+ {"[\\", "unfinished character class"},
+ }
+ for _, tt := range tests {
+ t.Run(tt.pattern, func(t *testing.T) {
+ _, err := Compile(tt.pattern)
+ if err == nil {
+ t.Fail()
+ } else if err.Error() != tt.msg {
+ t.Errorf("err = %v, want %v", err, tt.msg)
+ }
+ })
+ }
+}
+
+func Test_Intersect(t *testing.T) {
+ tests := []struct {
+ pattern1 string
+ pattern2 string
+ str string
+ matches bool
+ canMatch bool
+ }{
+ {"N-*", "N-*", "N-*", true, true},
+ {"N-9.99.*", "N-[1-9].*", "", false, true},
+ {"N-9.99.*", "N-[1-9][0-9].*", "", false, false},
+ }
+ for _, tt := range tests {
+ t.Run(tt.str, func(t *testing.T) {
+ a1, err1 := Compile(tt.pattern1)
+ a2, err2 := Compile(tt.pattern2)
+ if err1 != nil {
+ t.Fatal(err1)
+ }
+ if err2 != nil {
+ t.Fatal(err2)
+ }
+ a := Intersect(a1, a2)
+ matches := a.Match(tt.str)
+ if matches != tt.matches {
+ t.Errorf("Match() = %v, want %v", matches, tt.matches)
+ }
+ canMatch := a.CanMatch()
+ if canMatch != tt.canMatch {
+ t.Errorf("CanMatch() = %v, want %v", canMatch, tt.canMatch)
+ }
+ })
+ }
+}
Home |
Main Index |
Thread Index |
Old Index