Skip to content

Commit e039bec

Browse files
eregoneregon
committed
Ensure all visit* methods are overridden in YARPTranslator
* This is a good way to be notified when new node types are added.
1 parent 7f2084a commit e039bec

File tree

2 files changed

+111
-12
lines changed

2 files changed

+111
-12
lines changed

‎src/main/java/org/truffleruby/parser/YARPTranslator.java

Lines changed: 93 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ public RubyNode visitAliasMethodNode(Nodes.AliasMethodNode node) {
235235

236236
@Override
237237
public RubyNode visitAlternationPatternNode(Nodes.AlternationPatternNode node) {
238-
return defaultVisit(node);
238+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
239239
}
240240

241241
@Override
@@ -276,7 +276,17 @@ public RubyNode visitArrayNode(Nodes.ArrayNode node) {
276276

277277
@Override
278278
public RubyNode visitArrayPatternNode(Nodes.ArrayPatternNode node) {
279-
return defaultVisit(node);
279+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
280+
}
281+
282+
@Override
283+
public RubyNode visitAssocNode(Nodes.AssocNode node) {
284+
throw CompilerDirectives.shouldNotReachHere("handled in visitHashNode/getKeywordArgumentsDescriptor");
285+
}
286+
287+
@Override
288+
public RubyNode visitAssocSplatNode(Nodes.AssocSplatNode node) {
289+
throw CompilerDirectives.shouldNotReachHere("handled in visitHashNode/getKeywordArgumentsDescriptor");
280290
}
281291

282292
@Override
@@ -440,7 +450,7 @@ private RescueNode translateExceptionNodes(ArrayList<Nodes.Node> exceptionNodes,
440450

441451
@Override
442452
public RubyNode visitBlockLocalVariableNode(Nodes.BlockLocalVariableNode node) {
443-
return super.visitBlockLocalVariableNode(node);
453+
throw CompilerDirectives.shouldNotReachHere("handled in translateBlockAndLambda");
444454
}
445455

446456
@Override
@@ -467,6 +477,7 @@ private RubyNode translateBlockAndLambda(Nodes.Node node, Nodes.Node parametersN
467477

468478
final Nodes.ParametersNode parameters;
469479
if (parametersNode instanceof Nodes.BlockParametersNode blockParameters) {
480+
// NOTE: we ignore BlockParametersNode#locals, it is fully redundant with BlockNode#locals/LambdaNode#locals
470481
parameters = blockParameters.parameters != null ? blockParameters.parameters : ZERO_PARAMETERS_NODE;
471482
} else if (parametersNode instanceof Nodes.NumberedParametersNode numberedParameters) {
472483
// build Nodes.BlockParametersNode with required parameters _1, _2, etc
@@ -553,6 +564,16 @@ public RubyNode visitBlockArgumentNode(Nodes.BlockArgumentNode node) {
553564
return assignPositionAndFlags(node, rubyNode);
554565
}
555566

567+
@Override
568+
public RubyNode visitBlockParameterNode(Nodes.BlockParameterNode node) {
569+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
570+
}
571+
572+
@Override
573+
public RubyNode visitBlockParametersNode(Nodes.BlockParametersNode node) {
574+
throw CompilerDirectives.shouldNotReachHere("handled in translateBlockAndLambda");
575+
}
576+
556577
@Override
557578
public RubyNode visitBreakNode(Nodes.BreakNode node) {
558579
// detect syntax error
@@ -898,7 +919,7 @@ public RubyNode visitCallTargetNode(Nodes.CallTargetNode node) {
898919

899920
@Override
900921
public RubyNode visitCapturePatternNode(Nodes.CapturePatternNode node) {
901-
return defaultVisit(node);
922+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
902923
}
903924

904925
@Override
@@ -1574,7 +1595,7 @@ public RubyNode visitFalseNode(Nodes.FalseNode node) {
15741595

15751596
@Override
15761597
public RubyNode visitFindPatternNode(Nodes.FindPatternNode node) {
1577-
return defaultVisit(node);
1598+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
15781599
}
15791600

15801601
@Override
@@ -1752,6 +1773,11 @@ public RubyNode visitForwardingArgumentsNode(Nodes.ForwardingArgumentsNode node)
17521773
throw CompilerDirectives.shouldNotReachHere("handled in visitCallNode");
17531774
}
17541775

1776+
@Override
1777+
public RubyNode visitForwardingParameterNode(Nodes.ForwardingParameterNode node) {
1778+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
1779+
}
1780+
17551781
@Override
17561782
public RubyNode visitForwardingSuperNode(Nodes.ForwardingSuperNode node) {
17571783
var argumentsAndBlock = translateArgumentsAndBlock(null, node.block, environment.getMethodName());
@@ -1945,7 +1971,7 @@ public RubyNode visitHashNode(Nodes.HashNode node) {
19451971

19461972
@Override
19471973
public RubyNode visitHashPatternNode(Nodes.HashPatternNode node) {
1948-
return defaultVisit(node);
1974+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
19491975
}
19501976

19511977
@Override
@@ -1992,12 +2018,12 @@ public RubyNode visitImplicitNode(Nodes.ImplicitNode node) {
19922018

19932019
@Override
19942020
public RubyNode visitImplicitRestNode(Nodes.ImplicitRestNode node) {
1995-
return super.visitImplicitRestNode(node);
2021+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
19962022
}
19972023

19982024
@Override
19992025
public RubyNode visitInNode(Nodes.InNode node) {
2000-
return defaultVisit(node);
2026+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
20012027
}
20022028

20032029
@Override
@@ -2426,6 +2452,11 @@ public RubyNode visitKeywordHashNode(Nodes.KeywordHashNode node) {
24262452
return hash.accept(this);
24272453
}
24282454

2455+
@Override
2456+
public RubyNode visitKeywordRestParameterNode(Nodes.KeywordRestParameterNode node) {
2457+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2458+
}
2459+
24292460
@Override
24302461
public RubyNode visitLambdaNode(Nodes.LambdaNode node) {
24312462
return translateBlockAndLambda(node, node.parameters, node.body, node.locals, null);
@@ -2615,7 +2646,7 @@ private RubyNode match2NonNilSetter(String name, int tempSlot, SourceIndexLength
26152646

26162647
@Override
26172648
public RubyNode visitMissingNode(Nodes.MissingNode node) {
2618-
return defaultVisit(node);
2649+
throw fail(node);
26192650
}
26202651

26212652
@Override
@@ -2646,6 +2677,11 @@ public RubyNode visitMultiWriteNode(Nodes.MultiWriteNode node) {
26462677
return assignPositionAndFlags(node, rubyNode);
26472678
}
26482679

2680+
@Override
2681+
public RubyNode visitMultiTargetNode(Nodes.MultiTargetNode node) {
2682+
throw CompilerDirectives.shouldNotReachHere("handled in visitForNode and other places");
2683+
}
2684+
26492685
@Override
26502686
public RubyNode visitNextNode(Nodes.NextNode node) {
26512687
// detect syntax error
@@ -2681,6 +2717,16 @@ public RubyNode visitNilNode(Nodes.NilNode node) {
26812717
return assignPositionAndFlags(node, rubyNode);
26822718
}
26832719

2720+
@Override
2721+
public RubyNode visitNoKeywordsParameterNode(Nodes.NoKeywordsParameterNode node) {
2722+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2723+
}
2724+
2725+
@Override
2726+
public RubyNode visitNumberedParametersNode(Nodes.NumberedParametersNode node) {
2727+
throw CompilerDirectives.shouldNotReachHere("handled in translateBlockAndLambda");
2728+
}
2729+
26842730
@Override
26852731
public RubyNode visitNumberedReferenceReadNode(Nodes.NumberedReferenceReadNode node) {
26862732
final RubyNode lastMatchNode = ReadGlobalVariableNodeGen.create("$~");
@@ -2689,6 +2735,16 @@ public RubyNode visitNumberedReferenceReadNode(Nodes.NumberedReferenceReadNode n
26892735
return assignPositionAndFlags(node, rubyNode);
26902736
}
26912737

2738+
@Override
2739+
public RubyNode visitOptionalKeywordParameterNode(Nodes.OptionalKeywordParameterNode node) {
2740+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2741+
}
2742+
2743+
@Override
2744+
public RubyNode visitOptionalParameterNode(Nodes.OptionalParameterNode node) {
2745+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2746+
}
2747+
26922748
@Override
26932749
public RubyNode visitOrNode(Nodes.OrNode node) {
26942750
final RubyNode left = node.left.accept(this);
@@ -2698,6 +2754,11 @@ public RubyNode visitOrNode(Nodes.OrNode node) {
26982754
return assignPositionAndFlags(node, rubyNode);
26992755
}
27002756

2757+
@Override
2758+
public RubyNode visitParametersNode(Nodes.ParametersNode node) {
2759+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2760+
}
2761+
27012762
@Override
27022763
public RubyNode visitParenthesesNode(Nodes.ParenthesesNode node) {
27032764
if (node.body == null) {
@@ -2709,12 +2770,12 @@ public RubyNode visitParenthesesNode(Nodes.ParenthesesNode node) {
27092770

27102771
@Override
27112772
public RubyNode visitPinnedExpressionNode(Nodes.PinnedExpressionNode node) {
2712-
return defaultVisit(node);
2773+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
27132774
}
27142775

27152776
@Override
27162777
public RubyNode visitPinnedVariableNode(Nodes.PinnedVariableNode node) {
2717-
return defaultVisit(node);
2778+
throw CompilerDirectives.shouldNotReachHere("handled in YARPPatternMatchingTranslator");
27182779
}
27192780

27202781
@Override
@@ -2921,6 +2982,16 @@ private RaiseException regexpErrorToSyntaxError(DeferredRaiseException dre, Node
29212982
}
29222983
}
29232984

2985+
@Override
2986+
public RubyNode visitRequiredKeywordParameterNode(Nodes.RequiredKeywordParameterNode node) {
2987+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2988+
}
2989+
2990+
@Override
2991+
public RubyNode visitRequiredParameterNode(Nodes.RequiredParameterNode node) {
2992+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
2993+
}
2994+
29242995
@Override
29252996
public RubyNode visitRescueModifierNode(Nodes.RescueModifierNode node) {
29262997
// use Ruby StandardError class as far as exception class cannot be specified
@@ -2941,7 +3012,12 @@ public RubyNode visitRescueModifierNode(Nodes.RescueModifierNode node) {
29413012

29423013
@Override
29433014
public RubyNode visitRescueNode(Nodes.RescueNode node) {
2944-
return defaultVisit(node);
3015+
throw CompilerDirectives.shouldNotReachHere("handled in visitBeginNode");
3016+
}
3017+
3018+
@Override
3019+
public RubyNode visitRestParameterNode(Nodes.RestParameterNode node) {
3020+
throw CompilerDirectives.shouldNotReachHere("handled in YARPLoadArgumentsTranslator");
29453021
}
29463022

29473023
@Override
@@ -3190,6 +3266,11 @@ public RubyNode visitUntilNode(Nodes.UntilNode node) {
31903266
return assignPositionAndFlags(node, rubyNode);
31913267
}
31923268

3269+
@Override
3270+
public RubyNode visitWhenNode(Nodes.WhenNode node) {
3271+
throw CompilerDirectives.shouldNotReachHere("handled in visitCaseNode");
3272+
}
3273+
31933274
@Override
31943275
public RubyNode visitWhileNode(Nodes.WhileNode node) {
31953276
final RubyNode rubyNode = translateWhileNode(node, node.predicate, node.statements, false,

‎tool/jt.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2889,6 +2889,19 @@ def final_classes
28892889
end
28902890
end
28912891

2892+
# This is a good way to be notified when new node types are added in Prism,
2893+
# and helps to ensure we handle all nodes.
2894+
def all_visit_methods_in_yarp_translator
2895+
visit_methods = File.read('src/yarp/java/org/prism/AbstractNodeVisitor.java').scan(/\bpublic T (visit\w+)\(/).map(&:first)
2896+
overridden_methods = File.read('src/main/java/org/truffleruby/parser/YARPTranslator.java').scan(/\bpublic \w*Node (visit\w+)\(/).map(&:first)
2897+
missing = visit_methods - overridden_methods
2898+
if missing.empty?
2899+
false
2900+
else
2901+
STDERR.puts "Missing methods in YARPTranslator: #{missing}"
2902+
end
2903+
end
2904+
28922905
private
28932906

28942907
def split_arguments(line)
@@ -3016,13 +3029,18 @@ def final_classes
30163029
Formatting.final_classes
30173030
end
30183031

3032+
def all_visit_methods_in_yarp_translator
3033+
Formatting.all_visit_methods_in_yarp_translator
3034+
end
3035+
30193036
def format_specializations_check
30203037
any_failed = false
30213038
[
30223039
[format_specializations_visibility, 'Some Specializations used public/protected/private visibility modifier.'],
30233040
[format_specializations_arguments, 'Some Specializations were not properly formatted.'],
30243041
[Formatting.format_imports, 'There were extra blank lines around imports.'],
30253042
[final_classes, 'There were classes which should be marked as final but were not.'],
3043+
[all_visit_methods_in_yarp_translator, 'YARPTranslator should override all visit* methods from AbstractNodeVisitor.']
30263044
].each do |changed, error_message|
30273045
any_failed ||= changed
30283046
$stderr.puts error_message if changed

0 commit comments

Comments
 (0)