Skip to content

Commit c589cf0

Browse files
committed
Add warning for rules mixing positional and named references
1 parent b5eba1e commit c589cf0

4 files changed

Lines changed: 538 additions & 0 deletions

File tree

lib/lrama/warnings.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
require_relative 'warnings/conflicts'
55
require_relative 'warnings/implicit_empty'
6+
require_relative 'warnings/mixed_references'
67
require_relative 'warnings/name_conflicts'
78
require_relative 'warnings/redefined_rules'
89
require_relative 'warnings/required'
@@ -14,6 +15,7 @@ class Warnings
1415
def initialize(logger, warnings)
1516
@conflicts = Conflicts.new(logger, warnings)
1617
@implicit_empty = ImplicitEmpty.new(logger, warnings)
18+
@mixed_references = MixedReferences.new(logger, warnings)
1719
@name_conflicts = NameConflicts.new(logger, warnings)
1820
@redefined_rules = RedefinedRules.new(logger, warnings)
1921
@required = Required.new(logger, warnings)
@@ -24,6 +26,7 @@ def initialize(logger, warnings)
2426
def warn(grammar, states)
2527
@conflicts.warn(states)
2628
@implicit_empty.warn(grammar)
29+
@mixed_references.warn(grammar)
2730
@name_conflicts.warn(grammar)
2831
@redefined_rules.warn(grammar)
2932
@required.warn(grammar)
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# rbs_inline: enabled
2+
# frozen_string_literal: true
3+
4+
module Lrama
5+
class Warnings
6+
# Warning rationale: Mixing positional and named references in one rule
7+
# - It reduces readability and makes semantic actions harder to maintain
8+
# - Named references are generally more robust when RHS changes
9+
# Scope:
10+
# - This warning only targets semantic value references (`$...`)
11+
# - It intentionally ignores location references (`@...`),
12+
# index references (`$:...`), and special LHS references (`$$`)
13+
class MixedReferences
14+
# @rbs (Lrama::Logger logger, bool warnings) -> void
15+
def initialize(logger, warnings)
16+
@logger = logger
17+
@warnings = warnings
18+
end
19+
20+
# @rbs (Lrama::Grammar grammar) -> void
21+
def warn(grammar)
22+
return unless @warnings
23+
24+
build_grouped_usage(grammar).each do |rule, usage|
25+
next unless usage[:positional] && usage[:named]
26+
27+
@logger.warn("warning: rule `#{rule.as_comment}` mixes positional and named references; use named references consistently")
28+
end
29+
end
30+
31+
private
32+
33+
# @rbs (Lrama::Grammar grammar) -> Hash[Lrama::Grammar::Rule, { positional: bool, named: bool }]
34+
def build_grouped_usage(grammar)
35+
grouped_usage = {} #: Hash[Lrama::Grammar::Rule, { positional: bool, named: bool }]
36+
37+
grammar.rules.each do |rule|
38+
next unless (token_code = rule.token_code)
39+
40+
original_rule = rule.original_rule || rule
41+
usage = (grouped_usage[original_rule] ||= { positional: false, named: false })
42+
classify_references(token_code.references, token_code.s_value, usage)
43+
end
44+
45+
grouped_usage
46+
end
47+
48+
# @rbs (Array[Lrama::Grammar::Reference] references, String source, { positional: bool, named: bool } usage) -> void
49+
def classify_references(references, source, usage)
50+
references.each do |ref|
51+
if positional_reference?(ref)
52+
usage[:positional] = true
53+
elsif named_reference?(ref, source)
54+
usage[:named] = true
55+
end
56+
end
57+
end
58+
59+
# @rbs (Lrama::Grammar::Reference ref) -> bool
60+
def positional_reference?(ref)
61+
return false unless ref.type == :dollar
62+
return false if ref.index.nil?
63+
64+
ref.name.nil?
65+
end
66+
67+
# @rbs (Lrama::Grammar::Reference ref, String source) -> bool
68+
def named_reference?(ref, source)
69+
return false unless ref.type == :dollar
70+
71+
return true if !ref.name.nil? && ref.name != "$"
72+
73+
lhs_alias_reference?(ref, source)
74+
end
75+
76+
# @rbs (Lrama::Grammar::Reference ref, String source) -> bool
77+
def lhs_alias_reference?(ref, source)
78+
return false unless ref.name == "$"
79+
80+
lexeme = source[ref.first_column...ref.last_column]
81+
return false if lexeme.nil?
82+
83+
# Keep ignoring special LHS references ($$, $<tag>$), same as reference_to_c.
84+
return false if special_lhs_reference_lexeme?(lexeme)
85+
86+
# Treat remaining `$...` forms as named LHS aliases.
87+
true
88+
end
89+
90+
# @rbs (String lexeme) -> bool
91+
def special_lhs_reference_lexeme?(lexeme)
92+
lexeme == "$$" || (lexeme.start_with?("$<") && lexeme.end_with?("$"))
93+
end
94+
end
95+
end
96+
end

sig/generated/lrama/warnings/mixed_references.rbs

Lines changed: 40 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)