From 11c9c18de49a1935072790773bb306f20d4050fe Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 29 Oct 2020 13:04:26 +0000 Subject: [PATCH 1/2] Don't generate an index for optional fields that occur at most once --- extractor/src/extractor.rs | 37 ++++++++---- generator/src/main.rs | 68 +++++++++++----------- generator/src/ql_gen.rs | 42 +++++++++----- node-types/src/lib.rs | 22 +++++--- ql/src/codeql_ruby/ast.qll | 112 +++++++++++++++++-------------------- ql/src/ruby.dbscheme | 112 ++++++++++--------------------------- 6 files changed, 184 insertions(+), 209 deletions(-) diff --git a/extractor/src/extractor.rs b/extractor/src/extractor.rs index 7477806a644..8d98c61b04b 100644 --- a/extractor/src/extractor.rs +++ b/extractor/src/extractor.rs @@ -230,13 +230,19 @@ impl Visitor<'_> { ) } } - Storage::Table => { + Storage::Table(has_index) => { for (index, child_id) in child_ids.iter().enumerate() { + if !*has_index && index > 0 { + error!( + "saw multiple occurrences of field that should occur at most once" + ); + break; + } self.trap_output.push(TrapEntry::ChildOf( node_type_name(&field.parent.kind, field.parent.named), parent_id, field.get_name(), - Index(index), + if *has_index { Some(Index(index)) } else { None }, *child_id, )); } @@ -364,7 +370,7 @@ enum TrapEntry { // @node_def(self, arg?, location)@ Definition(String, Label, Vec, Label), // @node_child(self, index, parent)@ - ChildOf(String, Label, String, Index, Label), + ChildOf(String, Label, String, Option, Label), // @location(loc, path, r1, c1, r2, c2) Located(Vec), Comment(String), @@ -387,14 +393,23 @@ impl fmt::Display for TrapEntry { loc ) } - TrapEntry::ChildOf(pname, id, fname, idx, p) => write!( - f, - "{}({}, {}, {})", - escape_name(&format!("{}_{}", &pname, &fname)), - id, - idx, - p - ), + TrapEntry::ChildOf(pname, id, fname, idx, p) => match idx { + Some(idx) => write!( + f, + "{}({}, {}, {})", + escape_name(&format!("{}_{}", &pname, &fname)), + id, + idx, + p + ), + None => write!( + f, + "{}({}, {})", + escape_name(&format!("{}_{}", &pname, &fname)), + id, + p + ), + }, TrapEntry::Located(args) => write!( f, "location({}, {}, {}, {}, {}, {})", diff --git a/generator/src/main.rs b/generator/src/main.rs index 2b8350b0698..bdedcbdd938 100644 --- a/generator/src/main.rs +++ b/generator/src/main.rs @@ -49,45 +49,49 @@ fn add_field( ) { let field_name = field.get_name(); let parent_name = node_types::node_type_name(&field.parent.kind, field.parent.named); - match field.storage { - node_types::Storage::Table { .. } => { + match &field.storage { + node_types::Storage::Table(has_index) => { // This field can appear zero or multiple times, so put // it in an auxiliary table. let field_type = make_field_type(&parent_name, &field_name, &field.types, entries); + let parent_column = dbscheme::Column { + unique: !*has_index, + db_type: dbscheme::DbColumnType::Int, + name: node_types::escape_name(&parent_name), + ql_type: ql::Type::AtType(node_types::escape_name(&parent_name)), + ql_type_is_ref: true, + }; + let index_column = dbscheme::Column { + unique: false, + db_type: dbscheme::DbColumnType::Int, + name: "index".to_string(), + ql_type: ql::Type::Int, + ql_type_is_ref: true, + }; + let field_column = dbscheme::Column { + unique: true, + db_type: dbscheme::DbColumnType::Int, + name: node_types::escape_name(&field_type), + ql_type: ql::Type::AtType(field_type), + ql_type_is_ref: true, + }; let field_table = dbscheme::Table { name: format!("{}_{}", parent_name, field_name), - columns: vec![ - // First column is a reference to the parent. - dbscheme::Column { - unique: false, - db_type: dbscheme::DbColumnType::Int, - name: node_types::escape_name(&parent_name), - ql_type: ql::Type::AtType(node_types::escape_name(&parent_name)), - ql_type_is_ref: true, - }, - // Then an index column. - dbscheme::Column { - unique: false, - db_type: dbscheme::DbColumnType::Int, - name: "index".to_string(), - ql_type: ql::Type::Int, - ql_type_is_ref: true, - }, - // And then the field - dbscheme::Column { - unique: true, - db_type: dbscheme::DbColumnType::Int, - name: node_types::escape_name(&field_type), - ql_type: ql::Type::AtType(field_type), - ql_type_is_ref: true, - }, - ], + columns: if *has_index { + vec![parent_column, index_column, field_column] + } else { + vec![parent_column, field_column] + }, // In addition to the field being unique, the combination of // parent+index is unique, so add a keyset for them. - keysets: Some(vec![ - node_types::escape_name(&parent_name), - "index".to_string(), - ]), + keysets: if *has_index { + Some(vec![ + node_types::escape_name(&parent_name), + "index".to_string(), + ]) + } else { + None + }, }; entries.push(dbscheme::Entry::Table(field_table)); } diff --git a/generator/src/ql_gen.rs b/generator/src/ql_gen.rs index f400052e90d..d0251b92ee3 100644 --- a/generator/src/ql_gen.rs +++ b/generator/src/ql_gen.rs @@ -339,15 +339,21 @@ fn create_get_field_expr_for_column_storage( /// all indices. fn create_get_field_expr_for_table_storage( table_name: &str, - index_var_name: &str, + index_var_name: Option<&str>, ) -> ql::Expression { ql::Expression::Pred( table_name.to_owned(), - vec![ - ql::Expression::Var("this".to_owned()), - ql::Expression::Var(index_var_name.to_owned()), - ql::Expression::Var("result".to_owned()), - ], + match index_var_name { + Some(index_var_name) => vec![ + ql::Expression::Var("this".to_owned()), + ql::Expression::Var(index_var_name.to_owned()), + ql::Expression::Var("result".to_owned()), + ], + None => vec![ + ql::Expression::Var("this".to_owned()), + ql::Expression::Var("result".to_owned()), + ], + }, ) } @@ -400,7 +406,7 @@ fn create_field_getters( *main_table_column_index += 1; result } - node_types::Storage::Table => { + node_types::Storage::Table(has_index) => { let field_table_name = format!("{}_{}", parent_name, &field.get_name()); ( ql::Predicate { @@ -410,13 +416,23 @@ fn create_field_getters( ), overridden: false, return_type: Some(ql::Type::Normal(dbscheme_name_to_class_name(field_type))), - formal_parameters: vec![ql::FormalParameter { - name: "i".to_owned(), - param_type: ql::Type::Int, - }], - body: create_get_field_expr_for_table_storage(&field_table_name, "i"), + formal_parameters: if *has_index { + vec![ql::FormalParameter { + name: "i".to_owned(), + param_type: ql::Type::Int, + }] + } else { + vec![] + }, + body: create_get_field_expr_for_table_storage( + &field_table_name, + if *has_index { Some("i") } else { None }, + ), }, - create_get_field_expr_for_table_storage(&field_table_name, "_"), + create_get_field_expr_for_table_storage( + &field_table_name, + if *has_index { Some("_") } else { None }, + ), ) } } diff --git a/node-types/src/lib.rs b/node-types/src/lib.rs index d7b4128018d..fffd94c8d1d 100644 --- a/node-types/src/lib.rs +++ b/node-types/src/lib.rs @@ -46,8 +46,9 @@ impl Field { pub enum Storage { /// the field is stored as a column in the parent table Column, - // the field is store in a link table - Table, + /// the field is stored in a link table, and may or may not have an + /// associated index column + Table(bool), } pub fn read_node_types(node_types_path: &Path) -> std::io::Result> { @@ -122,16 +123,19 @@ fn add_field( field_info: &FieldInfo, fields: &mut Vec, ) { - let storage; - if !field_info.multiple && field_info.required { + let storage = if !field_info.multiple && field_info.required { // This field must appear exactly once, so we add it as // a column to the main table for the node type. - storage = Storage::Column; + Storage::Column + } else if !field_info.multiple { + // This field is optional but can occur at most once. Put it in an + // auxiliary table without an index. + Storage::Table(false) } else { - // This field can appear zero or multiple times, so put - // it in an auxiliary table. - storage = Storage::Table; - } + // This field can occur multiple times. Put it in an auxiliary table + // with an associated index. + Storage::Table(true) + }; fields.push(Field { parent: TypeName { kind: parent_type_name.kind.to_string(), diff --git a/ql/src/codeql_ruby/ast.qll b/ql/src/codeql_ruby/ast.qll index 6961412bf08..c0ae4a8398e 100644 --- a/ql/src/codeql_ruby/ast.qll +++ b/ql/src/codeql_ruby/ast.qll @@ -225,9 +225,9 @@ class Break extends @break, Top, ArgumentListChildType, ArrayChildType, Assignme override Location getLocation() { break_def(this, result) } - ArgumentList getChild(int i) { break_child(this, i, result) } + ArgumentList getChild() { break_child(this, result) } - override Top getAFieldOrChild() { break_child(this, _, result) } + override Top getAFieldOrChild() { break_child(this, result) } } class CallMethodType extends @call_method_type, Top { } @@ -257,11 +257,11 @@ class Case extends @case__, Top, UnderscorePrimary { override Location getLocation() { case_def(this, result) } - UnderscoreStatement getValue(int i) { case_value(this, i, result) } + UnderscoreStatement getValue() { case_value(this, result) } CaseChildType getChild(int i) { case_child(this, i, result) } - override Top getAFieldOrChild() { case_value(this, _, result) or case_child(this, _, result) } + override Top getAFieldOrChild() { case_value(this, result) or case_child(this, _, result) } } class ChainedString extends @chained_string, Top, UnderscorePrimary { @@ -385,11 +385,11 @@ class Else extends @else, Top, BeginChildType, CaseChildType, ClassChildType, Do override Location getLocation() { else_def(this, result) } - SemicolonUnnamed getCondition(int i) { else_condition(this, i, result) } + SemicolonUnnamed getCondition() { else_condition(this, result) } ElseChildType getChild(int i) { else_child(this, i, result) } - override Top getAFieldOrChild() { else_condition(this, _, result) or else_child(this, _, result) } + override Top getAFieldOrChild() { else_condition(this, result) or else_child(this, _, result) } } class ElsifAlternativeType extends @elsif_alternative_type, Top { } @@ -399,16 +399,14 @@ class Elsif extends @elsif, Top, ElsifAlternativeType, IfAlternativeType, Unless override Location getLocation() { elsif_def(this, _, result) } - ElsifAlternativeType getAlternative(int i) { elsif_alternative(this, i, result) } + ElsifAlternativeType getAlternative() { elsif_alternative(this, result) } UnderscoreStatement getCondition() { elsif_def(this, result, _) } - Then getConsequence(int i) { elsif_consequence(this, i, result) } + Then getConsequence() { elsif_consequence(this, result) } override Top getAFieldOrChild() { - elsif_alternative(this, _, result) or - elsif_def(this, result, _) or - elsif_consequence(this, _, result) + elsif_alternative(this, result) or elsif_def(this, result, _) or elsif_consequence(this, result) } } @@ -517,9 +515,9 @@ class HashSplatParameter extends @hash_splat_parameter, Top, BlockParametersChil override Location getLocation() { hash_splat_parameter_def(this, result) } - Identifier getName(int i) { hash_splat_parameter_name(this, i, result) } + Identifier getName() { hash_splat_parameter_name(this, result) } - override Top getAFieldOrChild() { hash_splat_parameter_name(this, _, result) } + override Top getAFieldOrChild() { hash_splat_parameter_name(this, result) } } class IfAlternativeType extends @if_alternative_type, Top { } @@ -529,14 +527,14 @@ class If extends @if, Top, UnderscorePrimary { override Location getLocation() { if_def(this, _, result) } - IfAlternativeType getAlternative(int i) { if_alternative(this, i, result) } + IfAlternativeType getAlternative() { if_alternative(this, result) } UnderscoreStatement getCondition() { if_def(this, result, _) } - Then getConsequence(int i) { if_consequence(this, i, result) } + Then getConsequence() { if_consequence(this, result) } override Top getAFieldOrChild() { - if_alternative(this, _, result) or if_def(this, result, _) or if_consequence(this, _, result) + if_alternative(this, result) or if_def(this, result, _) or if_consequence(this, result) } } @@ -585,10 +583,10 @@ class KeywordParameter extends @keyword_parameter, Top, BlockParametersChildType Identifier getName() { keyword_parameter_def(this, result, _) } - UnderscoreArg getValue(int i) { keyword_parameter_value(this, i, result) } + UnderscoreArg getValue() { keyword_parameter_value(this, result) } override Top getAFieldOrChild() { - keyword_parameter_def(this, result, _) or keyword_parameter_value(this, _, result) + keyword_parameter_def(this, result, _) or keyword_parameter_value(this, result) } } @@ -601,11 +599,9 @@ class Lambda extends @lambda, Top, UnderscorePrimary { LambdaBodyType getBody() { lambda_def(this, result, _) } - LambdaParameters getParameters(int i) { lambda_parameters(this, i, result) } + LambdaParameters getParameters() { lambda_parameters(this, result) } - override Top getAFieldOrChild() { - lambda_def(this, result, _) or lambda_parameters(this, _, result) - } + override Top getAFieldOrChild() { lambda_def(this, result, _) or lambda_parameters(this, result) } } class LambdaParametersChildType extends @lambda_parameters_child_type, Top { } @@ -641,14 +637,12 @@ class Method extends @method, Top, UnderscorePrimary { UnderscoreMethodName getName() { method_def(this, result, _) } - MethodParameters getParameters(int i) { method_parameters(this, i, result) } + MethodParameters getParameters() { method_parameters(this, result) } MethodChildType getChild(int i) { method_child(this, i, result) } override Top getAFieldOrChild() { - method_def(this, result, _) or - method_parameters(this, _, result) or - method_child(this, _, result) + method_def(this, result, _) or method_parameters(this, result) or method_child(this, _, result) } } @@ -665,15 +659,15 @@ class MethodCall extends @method_call, Top, ArgumentListChildType, ArrayChildTyp override Location getLocation() { method_call_def(this, _, result) } - ArgumentList getArguments(int i) { method_call_arguments(this, i, result) } + ArgumentList getArguments() { method_call_arguments(this, result) } - MethodCallBlockType getBlock(int i) { method_call_block(this, i, result) } + MethodCallBlockType getBlock() { method_call_block(this, result) } MethodCallMethodType getMethod() { method_call_def(this, result, _) } override Top getAFieldOrChild() { - method_call_arguments(this, _, result) or - method_call_block(this, _, result) or + method_call_arguments(this, result) or + method_call_block(this, result) or method_call_def(this, result, _) } } @@ -715,9 +709,9 @@ class Next extends @next, Top, ArgumentListChildType, ArrayChildType, Assignment override Location getLocation() { next_def(this, result) } - ArgumentList getChild(int i) { next_child(this, i, result) } + ArgumentList getChild() { next_child(this, result) } - override Top getAFieldOrChild() { next_child(this, _, result) } + override Top getAFieldOrChild() { next_child(this, result) } } class Operator extends @operator, Top, CallMethodType, UnderscoreMethodName { @@ -838,9 +832,9 @@ class Redo extends @redo, Top, UnderscorePrimary { override Location getLocation() { redo_def(this, result) } - ArgumentList getChild(int i) { redo_child(this, i, result) } + ArgumentList getChild() { redo_child(this, result) } - override Top getAFieldOrChild() { redo_child(this, _, result) } + override Top getAFieldOrChild() { redo_child(this, result) } } class RegexChildType extends @regex_child_type, Top { } @@ -861,16 +855,14 @@ class Rescue extends @rescue, Top, BeginChildType, ClassChildType, DoBlockChildT override Location getLocation() { rescue_def(this, result) } - Then getBody(int i) { rescue_body(this, i, result) } + Then getBody() { rescue_body(this, result) } - Exceptions getExceptions(int i) { rescue_exceptions(this, i, result) } + Exceptions getExceptions() { rescue_exceptions(this, result) } - ExceptionVariable getVariable(int i) { rescue_variable(this, i, result) } + ExceptionVariable getVariable() { rescue_variable(this, result) } override Top getAFieldOrChild() { - rescue_body(this, _, result) or - rescue_exceptions(this, _, result) or - rescue_variable(this, _, result) + rescue_body(this, result) or rescue_exceptions(this, result) or rescue_variable(this, result) } } @@ -896,9 +888,9 @@ class RestAssignment extends @rest_assignment, Top, DestructuredLeftAssignmentCh override Location getLocation() { rest_assignment_def(this, result) } - UnderscoreLhs getChild(int i) { rest_assignment_child(this, i, result) } + UnderscoreLhs getChild() { rest_assignment_child(this, result) } - override Top getAFieldOrChild() { rest_assignment_child(this, _, result) } + override Top getAFieldOrChild() { rest_assignment_child(this, result) } } class Retry extends @retry, Top, UnderscorePrimary { @@ -906,9 +898,9 @@ class Retry extends @retry, Top, UnderscorePrimary { override Location getLocation() { retry_def(this, result) } - ArgumentList getChild(int i) { retry_child(this, i, result) } + ArgumentList getChild() { retry_child(this, result) } - override Top getAFieldOrChild() { retry_child(this, _, result) } + override Top getAFieldOrChild() { retry_child(this, result) } } class Return extends @return, Top, ArgumentListChildType, ArrayChildType, AssignmentRightType, @@ -920,9 +912,9 @@ class Return extends @return, Top, ArgumentListChildType, ArrayChildType, Assign override Location getLocation() { return_def(this, result) } - ArgumentList getChild(int i) { return_child(this, i, result) } + ArgumentList getChild() { return_child(this, result) } - override Top getAFieldOrChild() { return_child(this, _, result) } + override Top getAFieldOrChild() { return_child(this, result) } } class RightAssignmentListChildType extends @right_assignment_list_child_type, Top { } @@ -947,10 +939,10 @@ class ScopeResolution extends @scope_resolution, Top, ClassNameType, MethodCallM ScopeResolutionNameType getName() { scope_resolution_def(this, result, _) } - UnderscorePrimary getScope(int i) { scope_resolution_scope(this, i, result) } + UnderscorePrimary getScope() { scope_resolution_scope(this, result) } override Top getAFieldOrChild() { - scope_resolution_def(this, result, _) or scope_resolution_scope(this, _, result) + scope_resolution_def(this, result, _) or scope_resolution_scope(this, result) } } @@ -993,14 +985,14 @@ class SingletonMethod extends @singleton_method, Top, UnderscorePrimary { SingletonMethodObjectType getObject() { singleton_method_def(this, _, result, _) } - MethodParameters getParameters(int i) { singleton_method_parameters(this, i, result) } + MethodParameters getParameters() { singleton_method_parameters(this, result) } SingletonMethodChildType getChild(int i) { singleton_method_child(this, i, result) } override Top getAFieldOrChild() { singleton_method_def(this, result, _, _) or singleton_method_def(this, _, result, _) or - singleton_method_parameters(this, _, result) or + singleton_method_parameters(this, result) or singleton_method_child(this, _, result) } } @@ -1023,9 +1015,9 @@ class SplatParameter extends @splat_parameter, Top, BlockParametersChildType, override Location getLocation() { splat_parameter_def(this, result) } - Identifier getName(int i) { splat_parameter_name(this, i, result) } + Identifier getName() { splat_parameter_name(this, result) } - override Top getAFieldOrChild() { splat_parameter_name(this, _, result) } + override Top getAFieldOrChild() { splat_parameter_name(this, result) } } class StringChildType extends @string_child_type, Top { } @@ -1137,16 +1129,16 @@ class Unless extends @unless, Top, UnderscorePrimary { override Location getLocation() { unless_def(this, _, result) } - UnlessAlternativeType getAlternative(int i) { unless_alternative(this, i, result) } + UnlessAlternativeType getAlternative() { unless_alternative(this, result) } UnderscoreStatement getCondition() { unless_def(this, result, _) } - Then getConsequence(int i) { unless_consequence(this, i, result) } + Then getConsequence() { unless_consequence(this, result) } override Top getAFieldOrChild() { - unless_alternative(this, _, result) or + unless_alternative(this, result) or unless_def(this, result, _) or - unless_consequence(this, _, result) + unless_consequence(this, result) } } @@ -1201,11 +1193,11 @@ class When extends @when, Top, CaseChildType { override Location getLocation() { when_def(this, result) } - Then getBody(int i) { when_body(this, i, result) } + Then getBody() { when_body(this, result) } WhenPatternType getPattern(int i) { when_pattern(this, i, result) } - override Top getAFieldOrChild() { when_body(this, _, result) or when_pattern(this, _, result) } + override Top getAFieldOrChild() { when_body(this, result) or when_pattern(this, _, result) } } class While extends @while, Top, UnderscorePrimary { @@ -1245,9 +1237,9 @@ class Yield extends @yield, Top, ArgumentListChildType, ArrayChildType, Assignme override Location getLocation() { yield_def(this, result) } - ArgumentList getChild(int i) { yield_child(this, i, result) } + ArgumentList getChild() { yield_child(this, result) } - override Top getAFieldOrChild() { yield_child(this, _, result) } + override Top getAFieldOrChild() { yield_child(this, result) } } class BangUnnamed extends @bang_unnamed, Top { diff --git a/ql/src/ruby.dbscheme b/ql/src/ruby.dbscheme index 6e65059c6b5..daa970ea1cd 100644 --- a/ql/src/ruby.dbscheme +++ b/ql/src/ruby.dbscheme @@ -182,10 +182,8 @@ block_parameters_def( int loc: @location ref ); -#keyset[break, index] break_child( - int break: @break ref, - int index: int ref, + unique int break: @break ref, unique int argument_list: @argument_list ref ); @@ -205,10 +203,8 @@ call_def( int loc: @location ref ); -#keyset[case__, index] case_value( - int case__: @case__ ref, - int index: int ref, + unique int case__: @case__ ref, unique int underscore_statement: @underscore_statement ref ); @@ -334,10 +330,8 @@ element_reference_def( int loc: @location ref ); -#keyset[else, index] else_condition( - int else: @else ref, - int index: int ref, + unique int else: @else ref, unique int semicolon_unnamed: @semicolon_unnamed ref ); @@ -357,17 +351,13 @@ else_def( @elsif_alternative_type = @else | @elsif -#keyset[elsif, index] elsif_alternative( - int elsif: @elsif ref, - int index: int ref, + unique int elsif: @elsif ref, unique int elsif_alternative_type: @elsif_alternative_type ref ); -#keyset[elsif, index] elsif_consequence( - int elsif: @elsif ref, - int index: int ref, + unique int elsif: @elsif ref, unique int then: @then ref ); @@ -467,10 +457,8 @@ hash_splat_argument_def( int loc: @location ref ); -#keyset[hash_splat_parameter, index] hash_splat_parameter_name( - int hash_splat_parameter: @hash_splat_parameter ref, - int index: int ref, + unique int hash_splat_parameter: @hash_splat_parameter ref, unique int identifier: @identifier ref ); @@ -481,17 +469,13 @@ hash_splat_parameter_def( @if_alternative_type = @else | @elsif -#keyset[if, index] if_alternative( - int if: @if ref, - int index: int ref, + unique int if: @if ref, unique int if_alternative_type: @if_alternative_type ref ); -#keyset[if, index] if_consequence( - int if: @if ref, - int index: int ref, + unique int if: @if ref, unique int then: @then ref ); @@ -522,10 +506,8 @@ interpolation_def( int loc: @location ref ); -#keyset[keyword_parameter, index] keyword_parameter_value( - int keyword_parameter: @keyword_parameter ref, - int index: int ref, + unique int keyword_parameter: @keyword_parameter ref, unique int underscore_arg: @underscore_arg ref ); @@ -537,10 +519,8 @@ keyword_parameter_def( @lambda_body_type = @block | @do_block -#keyset[lambda, index] lambda_parameters( - int lambda: @lambda ref, - int index: int ref, + unique int lambda: @lambda ref, unique int lambda_parameters: @lambda_parameters ref ); @@ -578,10 +558,8 @@ left_assignment_list_def( int loc: @location ref ); -#keyset[method, index] method_parameters( - int method: @method ref, - int index: int ref, + unique int method: @method ref, unique int method_parameters: @method_parameters ref ); @@ -600,19 +578,15 @@ method_def( int loc: @location ref ); -#keyset[method_call, index] method_call_arguments( - int method_call: @method_call ref, - int index: int ref, + unique int method_call: @method_call ref, unique int argument_list: @argument_list ref ); @method_call_block_type = @block | @do_block -#keyset[method_call, index] method_call_block( - int method_call: @method_call ref, - int index: int ref, + unique int method_call: @method_call ref, unique int method_call_block_type: @method_call_block_type ref ); @@ -655,10 +629,8 @@ module_def( int loc: @location ref ); -#keyset[next, index] next_child( - int next: @next ref, - int index: int ref, + unique int next: @next ref, unique int argument_list: @argument_list ref ); @@ -752,10 +724,8 @@ rational_def( int loc: @location ref ); -#keyset[redo, index] redo_child( - int redo: @redo ref, - int index: int ref, + unique int redo: @redo ref, unique int argument_list: @argument_list ref ); @@ -778,24 +748,18 @@ regex_def( int loc: @location ref ); -#keyset[rescue, index] rescue_body( - int rescue: @rescue ref, - int index: int ref, + unique int rescue: @rescue ref, unique int then: @then ref ); -#keyset[rescue, index] rescue_exceptions( - int rescue: @rescue ref, - int index: int ref, + unique int rescue: @rescue ref, unique int exceptions: @exceptions ref ); -#keyset[rescue, index] rescue_variable( - int rescue: @rescue ref, - int index: int ref, + unique int rescue: @rescue ref, unique int exception_variable: @exception_variable ref ); @@ -813,10 +777,8 @@ rescue_modifier_def( int loc: @location ref ); -#keyset[rest_assignment, index] rest_assignment_child( - int rest_assignment: @rest_assignment ref, - int index: int ref, + unique int rest_assignment: @rest_assignment ref, unique int underscore_lhs: @underscore_lhs ref ); @@ -825,10 +787,8 @@ rest_assignment_def( int loc: @location ref ); -#keyset[retry, index] retry_child( - int retry: @retry ref, - int index: int ref, + unique int retry: @retry ref, unique int argument_list: @argument_list ref ); @@ -837,10 +797,8 @@ retry_def( int loc: @location ref ); -#keyset[return, index] return_child( - int return: @return ref, - int index: int ref, + unique int return: @return ref, unique int argument_list: @argument_list ref ); @@ -865,10 +823,8 @@ right_assignment_list_def( @scope_resolution_name_type = @constant | @identifier -#keyset[scope_resolution, index] scope_resolution_scope( - int scope_resolution: @scope_resolution ref, - int index: int ref, + unique int scope_resolution: @scope_resolution ref, unique int underscore_primary: @underscore_primary ref ); @@ -901,10 +857,8 @@ singleton_class_def( @singleton_method_object_type = @underscore_arg | @underscore_variable -#keyset[singleton_method, index] singleton_method_parameters( - int singleton_method: @singleton_method ref, - int index: int ref, + unique int singleton_method: @singleton_method ref, unique int method_parameters: @method_parameters ref ); @@ -930,10 +884,8 @@ splat_argument_def( int loc: @location ref ); -#keyset[splat_parameter, index] splat_parameter_name( - int splat_parameter: @splat_parameter ref, - int index: int ref, + unique int splat_parameter: @splat_parameter ref, unique int identifier: @identifier ref ); @@ -1052,17 +1004,13 @@ undef_def( @unless_alternative_type = @else | @elsif -#keyset[unless, index] unless_alternative( - int unless: @unless ref, - int index: int ref, + unique int unless: @unless ref, unique int unless_alternative_type: @unless_alternative_type ref ); -#keyset[unless, index] unless_consequence( - int unless: @unless ref, - int index: int ref, + unique int unless: @unless ref, unique int then: @then ref ); @@ -1097,10 +1045,8 @@ until_modifier_def( int loc: @location ref ); -#keyset[when, index] when_body( - int when: @when ref, - int index: int ref, + unique int when: @when ref, unique int then: @then ref ); @@ -1134,10 +1080,8 @@ while_modifier_def( int loc: @location ref ); -#keyset[yield, index] yield_child( - int yield: @yield ref, - int index: int ref, + unique int yield: @yield ref, unique int argument_list: @argument_list ref ); From 547d12ca58f8fc9503dabf24202230a2d11cd299 Mon Sep 17 00:00:00 2001 From: Nick Rolfe Date: Thu, 29 Oct 2020 15:13:04 +0000 Subject: [PATCH 2/2] Add more info to error message --- extractor/src/extractor.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/extractor/src/extractor.rs b/extractor/src/extractor.rs index 8d98c61b04b..143aeccf8a8 100644 --- a/extractor/src/extractor.rs +++ b/extractor/src/extractor.rs @@ -234,7 +234,11 @@ impl Visitor<'_> { for (index, child_id) in child_ids.iter().enumerate() { if !*has_index && index > 0 { error!( - "saw multiple occurrences of field that should occur at most once" + "{}:{}: too many values for field: {}::{}", + &self.path, + node.start_position().row, + node.kind(), + &field.get_name() ); break; }