Skip to content

Commit

Permalink
Refactor and fix issue of handling colon in command subject.
Browse files Browse the repository at this point in the history 
The `to_command` function has been significantly refactored for better readability, maintainability, and error handling. The refactor includes breaking down the giant function into several smaller, more manageable helper functions. Most importantly, the issue of handling a colon ':' in the command subject, has been fixed to appropriately parse commands when the property is not declared. This ensures that the command subjects are correctly identified and extracted.
  • Loading branch information
@Barsik-sus
Barsik-sus committed Feb 2, 2024
1 parent 0da9710 commit 4112bad
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 127 deletions.
269 changes: 149 additions & 120 deletions module/move/wca/src/ca/grammar/converter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,78 +146,170 @@ pub( crate ) mod private
Ok( Namespace { commands } )
}

/// Converts raw command to grammatically correct
///
/// Make sure that this command is described in the grammar and matches it(command itself and all it options too).
pub fn to_command( &self, raw_command : RawCommand ) -> Result< GrammarCommand >
#[ cfg( feature = "on_unknown_command_error_suggest" ) ]
fn suggest_command( &self, user_input: &str ) -> Option< &str >
{
let variants = self
let jaro = eddie::JaroWinkler::new();
let sim = self
.commands
.get( &raw_command.name )
.ok_or_else::< error::for_app::Error, _ >
(
||
.iter()
.map( |( name, c )| ( jaro.similarity( name, user_input ), c ) )
.max_by( |( s1, _ ), ( s2, _ )| s1.total_cmp( s2 ) );
if let Some(( sim, variant )) = sim
{
if sim > 0.0
{
#[ cfg( feature = "on_unknown_command_error_suggest" ) ]
let phrase = &variant[ 0 ].phrase;
return Some( phrase );
}
}

None
}

fn find_variant< 'a >
(
variants: &'a [ Command ],
raw_command : &RawCommand,
) -> Option< &'a Command >
{
let mut maybe_valid_variants = vec![];

for variant @ Command
{
subjects,
properties,
properties_aliases,
..
}
in variants
{
let raw_subjects_count = raw_command.subjects.len();
let expected_subjects_count = subjects.len();
if raw_subjects_count > expected_subjects_count { continue; }

let mut maybe_subjects_count = 0_usize;
for ( k, _v ) in &raw_command.properties
{
if properties.contains_key( k ) { continue; }
if let Some( key ) = properties_aliases.get( k )
{
let jaro = eddie::JaroWinkler::new();
let sim = self
.commands
.iter()
.map( |( name, c )| ( jaro.similarity( name, &raw_command.name ), c ) )
.max_by( |( s1, _ ), ( s2, _ )| s1.total_cmp( s2 ) );
if let Some(( sim, variant )) = sim
{
if sim > 0.0
{
let phrase = &variant[ 0 ].phrase;
return err!( "Command not found. Maybe you mean `.{}`?", phrase );
}
}
if properties.contains_key( key ) { continue; }
}
err!( "Command not found. Please use `.` command to see the list of available commands. Sorry for the inconvenience. 😔" )
maybe_subjects_count += 1;
}
)?;

let mut cmd = None;
if raw_subjects_count + maybe_subjects_count > expected_subjects_count
{
continue;
}

maybe_valid_variants.push( variant );
}

// if maybe_valid_variants.len() == 1 { return Some( maybe_valid_variants[ 0 ] ) }
// qqq: provide better variant selection( E.g. based on types )
if !maybe_valid_variants.is_empty() { return Some( maybe_valid_variants[ 0 ] ) }
else { None }
}

fn extract_subjects( command : &Command, raw_command : &RawCommand, used_properties : &[ &String ] ) -> Result< Vec< Value > >
{
let mut subjects = vec![];

// find a variant that meets requirements
'variants_loop: for variant in variants
let all_subjects = raw_command
.subjects.clone().into_iter()
.chain
(
raw_command.properties.iter()
.filter( |( key, _ )| !used_properties.contains( key ) )
.map( |( key, value )| format!( "{key}:{value}" ) )
)
.collect::< Vec< _ > >();
let mut rc_subjects_iter = all_subjects.iter();
let mut current = rc_subjects_iter.next();

for ValueDescription { kind, optional, .. } in &command.subjects
{
subjects.clear();
let value = match current.and_then( | v | kind.try_cast( v.clone() ).ok() )
{
Some( v ) => v,
None if *optional => continue,
_ => return Err( err!( "Missing not optional subject" ) ),
};
subjects.push( value );
current = rc_subjects_iter.next();
}
if let Some( value ) = current { return Err( err!( "Can not identify a subject: `{}`", value ) ) }

if raw_command.subjects.len() > variant.subjects.len() { continue; }
let mut rc_subjects_iter = raw_command.subjects.iter();
Ok( subjects )
}

let mut current = rc_subjects_iter.next();
// try to match subjects
'internal: for ValueDescription { kind, optional, .. } in &variant.subjects
{
let value = match current.and_then( | v | kind.try_cast( v.clone() ).ok() )
{
Some( v ) => v,
None if *optional => continue 'internal,
_ => continue 'variants_loop,
};
fn extract_properties( command: &Command, raw_command : HashMap< String, String > ) -> Result< HashMap< String, Value > >
{
raw_command.into_iter()
.filter_map
(
|( key, value )|
// try to find a key
if command.properties.contains_key( &key ) { Some( key ) }
else if let Some( original_key ) = command.properties_aliases.get( &key ) { Some( original_key.clone() ) }
else { None }
// give a description. unwrap is safe because previous checks
.map( | key | ( command.properties.get( &key ).unwrap(), key, value ) )
)
.map
(
|( value_description, key, value )|
value_description.kind.try_cast( value ).map( | v | ( key.clone(), v ) )
)
.collect::< Result< HashMap< _, _ > > >()
}

subjects.push( value );
current = rc_subjects_iter.next();
fn group_properties_and_their_aliases< 'a, Ks >( aliases : &'a HashMap< String, String >, used_keys : Ks ) -> Vec< &String >
where
Ks : Iterator< Item = &'a String >
{
let reverse_aliases = {
let mut map = HashMap::< &String, Vec< &String > >::new();
for ( property, alias ) in aliases
{
map.entry( alias ).or_default().push( property );
}
// if something exists after all expected subjects - this isn't correct variant
if current.is_some() { continue 'variants_loop; }
map
};

cmd = Some( variant );
}
used_keys.flat_map( | key |
{
reverse_aliases.get( key ).into_iter().flatten().map( | k | *k ).chain( Some( key ) )
})
.collect::< Vec< _ > >()
}

let Some( cmd ) = cmd else
/// Converts raw command to grammatically correct
///
/// Make sure that this command is described in the grammar and matches it(command itself and all it options too).
pub fn to_command( &self, raw_command : RawCommand ) -> Result< GrammarCommand >
{
let variants = self.commands.get( &raw_command.name )
.ok_or_else::< error::for_app::Error, _ >
(
||
{
#[ cfg( feature = "on_unknown_command_error_suggest" ) ]
if let Some( phrase ) = self.suggest_command( &raw_command.name )
{ return err!( "Command not found. Maybe you mean `.{}`?", phrase ) }
err!( "Command not found. Please use `.` command to see the list of available commands. Sorry for the inconvenience. 😔" )
}
)?;

let Some( cmd ) = Self::find_variant( variants, &raw_command ) else
{
error::for_app::bail!
(
"`{}` command with specified subjects not found. Available variants `{:#?}`",
&raw_command.name,
variants
.iter()
variants.iter()
.map
(
| x |
Expand All @@ -227,80 +319,17 @@ pub( crate ) mod private
&raw_command.name,
{
let variants = x.subjects.iter().filter( | x | !x.optional ).map( | x | format!( "{:?}", x.kind ) ).collect::< Vec< _ > >();
if variants.is_empty()
{
String::new()
}
else
{
variants.join( "" )
}
if variants.is_empty() { String::new() } else { variants.join( "" ) }
}
)
).collect::< Vec< _ > >()
)
.collect::< Vec< _ > >()
);
};

let properties = if cmd.properties.is_empty()
{
for ( key, value ) in &raw_command.properties
{
let subj = format!( "{key}:{value}" );
if cmd.subjects.len() > raw_command.subjects.len()
{
subjects.push( Value::String( subj ) );
}
else
{
return Err( err!( "Too many subjects provided. Something wrong here: {}", subj ) );
}
}

HashMap::new()
}
else
{
raw_command.properties
.clone()
.into_iter()
.map
(
|( key, value )|
// find a key
if cmd.properties.contains_key( &key ) { Ok( key ) }
else
{
if raw_command.properties.len() > cmd.properties.len()
{
let subj = format!( "{key}:{value}" );
subjects.push( Value::String( subj ) );
if cmd.properties.is_empty()
{
cmd.properties_aliases.get( &key ).cloned().ok_or_else( || err!( "property `{}` not found for command `.{}`", key, &raw_command.name ) )
}
else
{
Ok( cmd.properties.keys().next().unwrap().to_string() )
}
}
else
{
cmd.properties_aliases.get( &key ).cloned().ok_or_else( || err!( "property `{}` not found for command `.{}`", key, &raw_command.name ) )
}
}
// give a description
.map( | key | ( key.clone(), cmd.properties.get( &key ).unwrap(), value ) )
)
.collect::< Result< Vec< _ > > >()?
.into_iter()
// an error can be extended with the value's hint
.map
(
|( key, value_description, value )|
value_description.kind.try_cast( value ).map( | v | ( key.clone(), v ) )
)
.collect::< Result< HashMap< _, _ > > >()?
};
let properties = Self::extract_properties( cmd, raw_command.properties.clone() )?;
let used_properties_with_their_aliases = Self::group_properties_and_their_aliases( &cmd.properties_aliases, properties.keys() );
let subjects = Self::extract_subjects( cmd, &raw_command, &used_properties_with_their_aliases )?;

Ok( GrammarCommand
{
Expand Down
3 changes: 1 addition & 2 deletions module/move/wca/tests/inc/commands_aggregator/basic.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ tests_impls!
a_id!( grammar_command.subjects, vec![ TheModule::Value::String( "qwe:rty".into() ) ] );
}

// qqq: subject should be parsed if optional property is not specified
fn optional_prop_subject_with_colon()
fn optional_prop_subject_with_colon()
{
let grammar = GrammarConverter::former()
.command
Expand Down
11 changes: 6 additions & 5 deletions module/move/wca/tests/inc/grammar/from_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,23 @@ tests_impls!
a_id!( vec![ Value::String( "subject".to_string() ) ], grammar_command.subjects );
a_true!( grammar_command.properties.is_empty() );

// with more subjects that it is setted
// with more subjects that it is set
let raw_command = parser.command( ".command subject1 subject2" ).unwrap();

let grammar_command = grammar_converter.to_command( raw_command );
a_true!( grammar_command.is_err() );

// with subject and property that isn't declareted
// with subject and property that isn't declared
let raw_command = parser.command( ".command subject prop:value" ).unwrap();

a_true!( grammar_converter.to_command( raw_command ).is_err() );

// with property that isn't declareted and without subject
// subject with colon when property not declared
let raw_command = parser.command( ".command prop:value" ).unwrap();

let grammar_command = grammar_converter.to_command( raw_command );
a_true!( grammar_command.is_err() );
let grammar_command = grammar_converter.to_command( raw_command ).unwrap();
a_id!( vec![ Value::String( "prop:value".to_string() ) ], grammar_command.subjects );
a_true!( grammar_command.properties.is_empty() );
}

fn subject_type_check()
Expand Down

0 comments on commit 4112bad

Please sign in to comment.