feat: add *Grammar.IsLeftTerminating and fix *Grammar.IsValid panic on non-left terminating rule

Author: pandatix

dag.go

@@ -16,6 +16,12 @@ type Depgraph map[string]*node
 // It includes core rules only if necessary.
 func (g *Grammar) DependencyGraph() Depgraph {
 	graph := Depgraph{}
+	for _, corerule := range coreRules {
+		graph[corerule.Name] = &node{
+			Rulename:     corerule.Name,
+			Dependencies: getDependencies(corerule.Alternation),
+		}
+	}
 	for _, rule := range g.Rulemap {
 		graph[rule.Name] = &node{
 			Rulename:     rule.Name,
@@ -78,6 +84,139 @@ func (g *Grammar) IsDAG() bool {
 	return true
 }
 
+// RuleContainsCycle returns whether the rule contains a cycle or not.
+// It travels through the whole rule dependency graph, such that
+// it checks if the rule is cyclic AND if one of its dependency is too.
+//
+// WARNING: it is different than IsLeftTerminating, refer to its doc.
+func (g *Grammar) RuleContainsCycle(rulename string) (bool, error) {
+	// Check the rule exists
+	rule := GetRule(rulename, g.Rulemap)
+	if rule == nil {
+		return false, &ErrRuleNotFound{
+			Rulename: rulename,
+		}
+	}
+
+	// Get all SCCs
+	scc := &cycle{
+		index: 0,
+		stack: []*node{},
+		dg:    g.DependencyGraph(),
+	}
+	scc.find()
+
+	return ruleContainsCycle(scc.sccs, rulename), nil
+}
+
+// IsLeftTerminating returns whether the rule is not left terminating.
+// It travels through the whole rule dependency graph, such that
+// it checks if the rule has a way to left terminate.
+//
+// Notice that it depends on the ordering your grammar, which could be
+// illustrated by the ABNF rule "element" that begins with the alternation
+// of a "rulename", which is terminating, and not by "option" or "group"
+// which are not.
+//
+// WARNING: it is different than RuleContainsCycle, refer to its doc.
+func (g *Grammar) IsLeftTerminating(rulename string) (bool, error) {
+	// Check the rule exists
+	rule := GetRule(rulename, g.Rulemap)
+	if rule == nil {
+		return false, &ErrRuleNotFound{
+			Rulename: rulename,
+		}
+	}
+
+	// Stack has the same signature as a rulemap in order to use getRuleIn for simplicity
+	stack := map[string]*Rule{
+		rulename: rule,
+	}
+	return isAltLeftTerminating(g, stack, rule.Alternation), nil
+}
+
+func isAltLeftTerminating(g *Grammar, stack map[string]*Rule, alt Alternation) bool {
+	for _, con := range alt.Concatenations {
+		for _, rep := range con.Repetitions {
+			_, subIsOption := rep.Element.(ElemOption)
+			if rep.Min == 0 || subIsOption {
+				if !isElemLeftTerminating(g, stack, rep.Element) {
+					return false
+				}
+				continue
+			}
+			if !isElemLeftTerminating(g, stack, rep.Element) {
+				return false
+			}
+			break
+		}
+	}
+	return true
+}
+
+func isElemLeftTerminating(g *Grammar, stack map[string]*Rule, elem ElemItf) bool {
+	switch v := elem.(type) {
+	case ElemRulename:
+		ruleInStack := (getRuleIn(v.Name, stack) != nil)
+		if ruleInStack {
+			return false
+		}
+		rule := GetRule(v.Name, g.Rulemap)
+		return isAltLeftTerminating(g, stack, rule.Alternation)
+	case ElemOption:
+		return isAltLeftTerminating(g, stack, v.Alternation)
+	case ElemGroup:
+		return isAltLeftTerminating(g, stack, v.Alternation)
+	case ElemCharVal:
+		return len(v.Values) != 0
+	case ElemProseVal:
+		return len(v.values) != 0
+	}
+	return true
+}
+
+func ruleContainsCycle(sccs [][]*node, rulename string) bool {
+	// Find rulename's SCC
+	scc := ([]*node)(nil)
+	rulenode := (*node)(nil)
+	for _, s := range sccs {
+		if scc != nil {
+			break
+		}
+		for _, ss := range s {
+			if ss.Rulename == rulename {
+				rulenode = ss
+				scc = s
+				break
+			}
+		}
+	}
+
+	// Check if cyclic
+	dependsOn := false
+	for _, dep := range rulenode.Dependencies {
+		if dep == rulename {
+			dependsOn = true
+			break
+		}
+	}
+	if dependsOn || len(scc) != 1 {
+		// If it depends on itself or is part of an SCC, then is cylic
+		return true
+	}
+
+	// Propagate to deps
+	for _, dep := range rulenode.Dependencies {
+		if dep == rulename {
+			continue
+		}
+		if ruleContainsCycle(sccs, dep) {
+			return true
+		}
+	}
+	return false
+}
+
 type cycle struct {
 	index int
 	stack []*node

dag_test.go

@@ -50,3 +50,129 @@ func Test_U_IsDAG(t *testing.T) {
 		})
 	}
 }
+
+func Test_U_RuleContainsCycle(t *testing.T) {
+	t.Parallel()
+
+	var tests = map[string]struct {
+		Grammar     *goabnf.Grammar
+		Rulename    string
+		ExpectedRes bool
+		ExpectErr   bool
+	}{
+		"abnf": {
+			Grammar:     goabnf.ABNF,
+			Rulename:    "rulelist",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"a-self-left": {
+			Grammar:     mustGrammar("a = a \"a\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"a-self-right": {
+			Grammar:     mustGrammar("a = \"a\" a\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"ab": {
+			Grammar:     mustGrammar("a = b \"a\"\r\nb = a \"b\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+	}
+
+	for testname, tt := range tests {
+		t.Run(testname, func(t *testing.T) {
+			assert := assert.New(t)
+
+			res, err := tt.Grammar.RuleContainsCycle(tt.Rulename)
+			if (err != nil) != tt.ExpectErr {
+				t.Fatalf("Expected error: %t ; got %v", tt.ExpectErr, err)
+			}
+			assert.Equal(tt.ExpectedRes, res)
+		})
+	}
+}
+
+func Test_U_IsLeftTerminating(t *testing.T) {
+	t.Parallel()
+
+	var tests = map[string]struct {
+		Grammar     *goabnf.Grammar
+		Rulename    string
+		ExpectedRes bool
+		ExpectErr   bool
+	}{
+		"abnf": {
+			Grammar:     goabnf.ABNF,
+			Rulename:    "rulelist",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"a-left": {
+			Grammar:     mustGrammar("a = a \"a\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: false,
+			ExpectErr:   false,
+		},
+		"a-right": {
+			Grammar:     mustGrammar("a = \"a\" a\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"ab-left": {
+			Grammar:     mustGrammar("a = b \"a\"\r\nb = a \"b\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: false,
+			ExpectErr:   false,
+		},
+		"ab-right": {
+			Grammar:     mustGrammar("a = b \"a\"\r\nb = \"b\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+		"option-a-left": {
+			Grammar:     mustGrammar("a = [a] \"a\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: false,
+			ExpectErr:   false,
+		},
+		"option-a-right": {
+			Grammar:     mustGrammar("a = [\"a\"] a\r\n"),
+			Rulename:    "a",
+			ExpectedRes: false,
+			ExpectErr:   false,
+		},
+		"group-a-left": {
+			Grammar:     mustGrammar("a = (a) \"a\"\r\n"),
+			Rulename:    "a",
+			ExpectedRes: false,
+			ExpectErr:   false,
+		},
+		"group-a-right": {
+			Grammar:     mustGrammar("a = (\"a\") a\r\n"),
+			Rulename:    "a",
+			ExpectedRes: true,
+			ExpectErr:   false,
+		},
+	}
+
+	for testname, tt := range tests {
+		t.Run(testname, func(t *testing.T) {
+			assert := assert.New(t)
+
+			res, err := tt.Grammar.IsLeftTerminating(tt.Rulename)
+			if (err != nil) != tt.ExpectErr {
+				t.Fatalf("Expected error: %t ; got %v", tt.ExpectErr, err)
+			}
+			assert.Equal(tt.ExpectedRes, res)
+		})
+	}
+}

generate.go

@@ -142,7 +142,7 @@ func (opt thresholdOption) apply(opts *genOpts) {
 // checkCanGenerateSafely returns no error if the rule can be generated
 // safely i.e. if the rule can exist without infinite recursion.
 // Factually, it checks if all involved rules have no path v such that it
-// produces a cycle (v:rule-*->rulen) AND that this path is mandatory
+// produces a cycle (v:rule-*->rule) AND that this path is mandatory
 // (no option, no repetition with a minimum of zero).
 func checkCanGenerateSafely(g *Grammar, rulename string) error {
 	rule := GetRule(rulename, g.Rulemap)

generate_fuzz_test.go

@@ -18,11 +18,12 @@ func FuzzGenerate(f *testing.F) {
 			if len(out) != 0 {
 				t.Fatal("output should be empty in case of an error")
 			}
-		} else {
-			valid := goabnf.ABNF.IsValid(rulename, out)
-			if !valid {
-				t.Fatalf("generated output is invalid, out: %s", out)
-			}
+			return
+		}
+
+		valid, err := goabnf.ABNF.IsValid(rulename, out)
+		if err != nil || !valid {
+			t.Fatalf("generated output is invalid, out: %s", out)
 		}
 	})
 }

grammar.go

@@ -15,14 +15,16 @@ type Grammar struct {
 // IsValid checks there exist at least a path that completly consumes
 // input, hence is valide given this gramma and especially one of its
 // rule.
-//
-// XXX panic if involves unvaoidable recursive rule.
-//   - checkCanGenerateSafely is not sufficient as parseAlt will recurse without limitation
-//     (checks there are none unavoidable path on its road, not that it won't fall/recurse into it)
-//   - deepness is not sufficient as recursing could be very difficult to predict
-func (g *Grammar) IsValid(rulename string, input []byte, opts ...ParseOption) bool {
+func (g *Grammar) IsValid(rulename string, input []byte, opts ...ParseOption) (bool, error) {
+	lt, err := g.IsLeftTerminating(rulename)
+	if err != nil {
+		return false, err
+	}
+	if !lt {
+		return false, fmt.Errorf("rule %s is not left terminating thus can't be validated without the risk of infinite recursion", rulename)
+	}
 	paths, err := Parse(input, g, rulename, opts...)
-	return len(paths) != 0 && err == nil
+	return len(paths) != 0 && err == nil, nil
 }
 
 // String returns the representation of the grammar that is valid

grammar_fuzz_test.go

@@ -12,19 +12,19 @@ func FuzzParseABNF(f *testing.F) {
 	}
 
 	f.Fuzz(func(t *testing.T, input []byte) {
-		path, err := goabnf.ParseABNF(input)
+		grammar, err := goabnf.ParseABNF(input)
 
 		if err != nil {
-			if path != nil {
+			if grammar != nil {
 				t.Fatal("Expected no path when error")
 			}
 			if err, ok := err.(*goabnf.ErrMultipleSolutionsFound); ok {
 				t.Fatalf("For input %s, got error %s", input, err)
 			}
-		} else {
-			if path == nil {
-				t.Fatal("Expected a path when no error")
-			}
+			return
+		}
+		if grammar == nil {
+			t.Fatal("Expected a grammar when no error")
 		}
 	})
 }

utils.go

@@ -151,11 +151,13 @@ func pow(v, e int) int {
 // it is a core rule or present in the grammar, or nil if not found.
 // It validates the RFC 5234 Section 2.1 "rule names are case insensitive".
 func GetRule(rulename string, rulemap map[string]*Rule) *Rule {
-	for _, coreRule := range coreRules {
-		if strings.EqualFold(rulename, coreRule.Name) {
-			return coreRule
-		}
+	if rule := getRuleIn(rulename, coreRules); rule != nil {
+		return rule
 	}
+	return getRuleIn(rulename, rulemap)
+}
+
+func getRuleIn(rulename string, rulemap map[string]*Rule) *Rule {
 	for _, rule := range rulemap {
 		if strings.EqualFold(rulename, rule.Name) {
 			return rule