Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP propagagte index #521

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/models/default_configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ pub fn default_substitutions() -> Vec<(String, String)> {
vec![]
}

pub fn default_substitutions_int() -> Vec<(String, u8)> {
vec![]
}

pub fn default_delete_file_if_empty() -> bool {
true
}
Expand Down Expand Up @@ -111,6 +115,10 @@ pub fn default_replace_idx() -> u8 {
u8::MAX
}

pub fn default_replace_idx_of() -> String {
String::new()
}

pub fn default_replace() -> String {
String::new()
}
Expand Down
5 changes: 2 additions & 3 deletions src/models/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Copyright (c) 2023 Uber Technologies, Inc.
limitations under the License.
*/

use std::fmt;
use std::{fmt, collections::HashMap};

use colored::Colorize;
use getset::{Getters, MutGetters};
Expand Down Expand Up @@ -65,7 +65,6 @@ impl Edit {
}
#[cfg(test)]
pub(crate) fn delete_range(code: &str, replacement_range: Range) -> Self {
use std::collections::HashMap;
Self {
p_match: Match::new(
code[replacement_range.start_byte..replacement_range.end_byte].to_string(),
Expand Down Expand Up @@ -145,7 +144,7 @@ impl SourceCodeUnit {
.get_matches(rule, rule_store, node, recursive)
.first()
.map(|p_match| {
let replacement_string = rule.replace().instantiate(p_match.matches());
let replacement_string = rule.replace().instantiate(p_match.matches(), &HashMap::new());
let edit = Edit::new(
p_match.clone(),
replacement_string,
Expand Down
18 changes: 9 additions & 9 deletions src/models/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,21 +276,21 @@ pub use filter;

impl Instantiate for Filter {
/// Create a new filter from `self` by updating the all queries (i.e., `enclosing_node`, `not_enclosing_node`, `contains`, `not_contains`) based on the substitutions.
fn instantiate(&self, substitutions_for_holes: &HashMap<String, String>) -> Filter {
fn instantiate(&self, substitutions_for_holes: &HashMap<String, String>, int_substitutions: &HashMap<String, u8>) -> Filter {
Filter {
enclosing_node: self.enclosing_node().instantiate(substitutions_for_holes),
enclosing_node: self.enclosing_node().instantiate(substitutions_for_holes, int_substitutions),
outermost_enclosing_node: self
.outermost_enclosing_node()
.instantiate(substitutions_for_holes),
.instantiate(substitutions_for_holes, int_substitutions),
not_enclosing_node: self
.not_enclosing_node()
.instantiate(substitutions_for_holes),
.instantiate(substitutions_for_holes, int_substitutions),
not_contains: self
.not_contains()
.iter()
.map(|x| x.instantiate(substitutions_for_holes))
.map(|x| x.instantiate(substitutions_for_holes, int_substitutions))
.collect_vec(),
contains: self.contains().instantiate(substitutions_for_holes),
contains: self.contains().instantiate(substitutions_for_holes, int_substitutions),
at_least: self.at_least,
at_most: self.at_most,
child_count: self.child_count,
Expand All @@ -310,7 +310,7 @@ impl SourceCodeUnit {
rule
.filters()
.iter()
.all(|filter| self._check(filter.clone(), node, rule_store, &updated_substitutions))
.all(|filter| self._check(filter.clone(), node, rule_store, &updated_substitutions, &HashMap::new()))
}

/// Determines if the given `node` meets the conditions specified by the `filter`.
Expand All @@ -328,10 +328,10 @@ impl SourceCodeUnit {
/// If these conditions hold, the function returns true, indicating the `node` meets the `filter`'s criteria.
fn _check(
&self, filter: Filter, node: Node, rule_store: &mut RuleStore,
substitutions: &HashMap<String, String>,
substitutions: &HashMap<String, String>, int_substitutions: &HashMap<String, u8>
) -> bool {
let mut node_to_check = node;
let instantiated_filter = filter.instantiate(substitutions);
let instantiated_filter = filter.instantiate(substitutions, int_substitutions);

if *filter.child_count() != default_child_count() {
return node.named_child_count() == (*filter.child_count() as usize);
Expand Down
26 changes: 22 additions & 4 deletions src/models/piranha_arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ use super::{
default_dry_run, default_exclude, default_global_tag_prefix, default_include,
default_number_of_ancestors_in_parent_scope, default_path_to_codebase,
default_path_to_configurations, default_path_to_output_summaries, default_piranha_language,
default_rule_graph, default_substitutions, GO, JAVA, KOTLIN, PYTHON, SWIFT, TSX, TYPESCRIPT,
default_rule_graph, default_substitutions, default_substitutions_int, GO, JAVA, KOTLIN, PYTHON, SWIFT, TSX, TYPESCRIPT,
},
language::PiranhaLanguage,
rule_graph::{read_user_config_files, RuleGraph, RuleGraphBuilder},
source_code_unit::SourceCodeUnit,
};
use crate::utilities::{parse_glob_pattern, parse_key_val};
use crate::utilities::{parse_glob_pattern, parse_key_val, parse_key_val_int};
use clap::builder::TypedValueParser;
use clap::Parser;
use derive_builder::Builder;
Expand Down Expand Up @@ -70,12 +70,18 @@ pub struct PiranhaArguments {
#[clap(short = 't', long, default_value_t = default_code_snippet())]
code_snippet: String,

/// These substitutions instantiate the initial set of rules.
/// These string substitutions instantiate the initial set of rules.
/// Usage : -s stale_flag_name=SOME_FLAG -s namespace=SOME_NS1
#[builder(default = "default_substitutions()")]
#[clap(short = 's', value_parser = parse_key_val)]
substitutions: Vec<(String, String)>,

/// These integer substitutions instantiate the initial set of rules.
/// Usage : -i stale_flag_name=SOME_FLAG -s namespace=SOME_NS1
#[builder(default = "default_substitutions_int()")]
#[clap(short = 'i', value_parser = parse_key_val_int)]
substitutions_int: Vec<(String, u8)>,

/// Directory containing the configuration files - `rules.toml` and `edges.toml` (optional)
#[get = "pub"]
#[builder(default = "default_path_to_configurations()")]
Expand Down Expand Up @@ -178,7 +184,7 @@ impl PiranhaArguments {
#[new]
fn py_new(
language: String, path_to_codebase: Option<String>, include: Option<Vec<String>>,
exclude: Option<Vec<String>>, substitutions: Option<&PyDict>,
exclude: Option<Vec<String>>, substitutions: Option<&PyDict>, substitutions_int: Option<&PyDict>,
path_to_configurations: Option<String>, rule_graph: Option<RuleGraph>,
code_snippet: Option<String>, dry_run: Option<bool>, cleanup_comments: Option<bool>,
cleanup_comments_buffer: Option<i32>, number_of_ancestors_in_parent_scope: Option<u8>,
Expand All @@ -192,6 +198,12 @@ impl PiranhaArguments {
.collect_vec()
});

let subs_int: Vec<(String, u8)> = substitutions_int.map_or(vec![], |s| {
s.iter()
.map(|(k, v)| (k.to_string(), v.to_string().parse().unwrap()))
.collect_vec()
});

let rg = rule_graph.unwrap_or_else(|| RuleGraphBuilder::default().build());
PiranhaArgumentsBuilder::default()
.path_to_codebase(path_to_codebase.unwrap_or_else(default_path_to_codebase))
Expand All @@ -214,6 +226,7 @@ impl PiranhaArguments {
.code_snippet(code_snippet.unwrap_or_else(default_code_snippet))
.language(PiranhaLanguage::from(language.as_str()))
.substitutions(subs)
.substitutions_int(subs_int)
.dry_run(dry_run.unwrap_or_else(default_dry_run))
.cleanup_comments(cleanup_comments.unwrap_or_else(default_cleanup_comments))
.cleanup_comments_buffer(
Expand Down Expand Up @@ -244,6 +257,7 @@ impl PiranhaArguments {
PiranhaArgumentsBuilder::default()
.path_to_codebase(p.path_to_codebase().to_string())
.substitutions(p.substitutions.clone())
.substitutions_int(p.substitutions_int.clone())
.language(p.language().clone())
.path_to_configurations(p.path_to_configurations().to_string())
.path_to_output_summary(p.path_to_output_summary().clone())
Expand All @@ -260,6 +274,10 @@ impl PiranhaArguments {
pub(crate) fn input_substitutions(&self) -> HashMap<String, String> {
self.substitutions.iter().cloned().collect()
}

pub(crate) fn input_int_substitutions(&self) -> HashMap<String, u8> {
self.substitutions_int.iter().cloned().collect()
}
}

impl PiranhaArgumentsBuilder {
Expand Down
45 changes: 37 additions & 8 deletions src/models/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use crate::utilities::{gen_py_str_methods, tree_sitter_utilities::TSQuery, Insta
use super::{
default_configs::{
default_filters, default_groups, default_holes, default_is_seed_rule, default_query,
default_replace, default_replace_idx, default_replace_node, default_rule_name,
default_replace, default_replace_idx, default_replace_idx_of, default_replace_node,
default_rule_name,
},
filter::Filter,
Validator,
Expand Down Expand Up @@ -62,6 +63,12 @@ pub struct Rule {
#[get = "pub"]
#[pyo3(get)]
replace_idx: u8,
/// The ith (i is the int value corresponding to the tag provided) child of node corresponding to the replace_node tag will be replaced
#[builder(default = "default_replace_idx_of()")]
#[serde(default = "default_replace_idx_of")]
#[get = "pub"]
#[pyo3(get)]
replace_idx_of: String,
/// Replacement pattern
#[builder(default = "default_replace()")]
#[serde(default = "default_replace")]
Expand Down Expand Up @@ -134,6 +141,7 @@ macro_rules! piranha_rule {
$(, query =$query: expr)?
$(, replace_node = $replace_node:expr)?
$(, replace_idx = $replace_idx:expr)?
$(, replace_idx_of = $replace_idx_of:expr)?
$(, replace = $replace:expr)?
$(, holes = [$($hole: expr)*])?
$(, is_seed_rule = $is_seed_rule:expr)?
Expand All @@ -145,6 +153,7 @@ macro_rules! piranha_rule {
$(.query($crate::utilities::tree_sitter_utilities::TSQuery::new($query.to_string())))?
$(.replace_node($replace_node.to_string()))?
$(.replace_idx($replace_idx.to_string()))?
$(.replace_idx_of($replace_idx_of.to_string()))?
$(.replace($replace.to_string()))?
$(.holes(std::collections::HashSet::from([$($hole.to_string(),)*])))?
$(.groups(std::collections::HashSet::from([$($group_name.to_string(),)*])))?
Expand All @@ -158,8 +167,8 @@ impl Rule {
#[new]
fn py_new(
name: String, query: Option<String>, replace: Option<String>, replace_idx: Option<u8>,
replace_node: Option<String>, holes: Option<HashSet<String>>, groups: Option<HashSet<String>>,
filters: Option<HashSet<Filter>>, is_seed_rule: Option<bool>,
replace_idx_of: Option<String>, replace_node: Option<String>, holes: Option<HashSet<String>>,
groups: Option<HashSet<String>>, filters: Option<HashSet<Filter>>, is_seed_rule: Option<bool>,
) -> Self {
let mut rule_builder = RuleBuilder::default();

Expand All @@ -176,6 +185,10 @@ impl Rule {
rule_builder.replace_idx(replace_idx);
}

if let Some(replace_idx_of) = replace_idx_of {
rule_builder.replace_idx_of(replace_idx_of);
}

if let Some(replace_node) = replace_node {
rule_builder.replace_node(replace_node);
}
Expand Down Expand Up @@ -223,7 +236,9 @@ pub(crate) struct InstantiatedRule {
}

impl InstantiatedRule {
pub(crate) fn new(rule: &Rule, substitutions: &HashMap<String, String>) -> Self {
pub(crate) fn new(
rule: &Rule, substitutions: &HashMap<String, String>, int_substitutions: &HashMap<String, u8>,
) -> Self {
let substitutions_for_holes: HashMap<String, String> = rule
.holes()
.iter()
Expand All @@ -236,7 +251,7 @@ impl InstantiatedRule {
panic!("{}", format!( "Could not instantiate the rule {rule:?} with substitutions {substitutions_for_holes:?}").red());
}
InstantiatedRule {
rule: rule.instantiate(&substitutions_for_holes),
rule: rule.instantiate(&substitutions_for_holes, int_substitutions),
substitutions: substitutions_for_holes,
}
}
Expand Down Expand Up @@ -280,11 +295,25 @@ impl Instantiate for Rule {
/// Create a new query from `self` by updating the `query` and `replace` based on the substitutions.
/// This functions assumes that each hole in the rule can be substituted.
/// i.e. It assumes that `substitutions_for_holes` is exhaustive and complete
fn instantiate(&self, substitutions_for_holes: &HashMap<String, String>) -> Rule {
fn instantiate(
&self, substitutions_for_holes: &HashMap<String, String>,
int_substitutions: &HashMap<String, u8>,
) -> Rule {
let updated_rule = self.clone();
Rule {
query: updated_rule.query().instantiate(substitutions_for_holes),
replace: updated_rule.replace().instantiate(substitutions_for_holes),
query: updated_rule
.query()
.instantiate(substitutions_for_holes, int_substitutions),
replace: updated_rule
.replace()
.instantiate(substitutions_for_holes, int_substitutions),
replace_idx: if *updated_rule.replace_idx_of() != default_replace_idx_of() {
*int_substitutions
.get(updated_rule.replace_idx_of())
.unwrap()
} else {
default_replace_idx()
},
..updated_rule
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/models/rule_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ impl RuleGraph {
// Group the next rules based on the scope
next_rules.collect(
String::from(&scope),
InstantiatedRule::new(to_rule_name, tag_matches),
InstantiatedRule::new(to_rule_name, tag_matches, &HashMap::new()),
);
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/models/rule_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,11 @@ impl RuleStore {

for rule in args.rule_graph().rules().clone() {
if *rule.is_seed_rule() {
rule_store.add_to_global_rules(&InstantiatedRule::new(&rule, &args.input_substitutions()));
rule_store.add_to_global_rules(&InstantiatedRule::new(
&rule,
&args.input_substitutions(),
&args.input_int_substitutions(),
));
}
}
trace!("Rule Store {}", format!("{rule_store:#?}"));
Expand Down
4 changes: 3 additions & 1 deletion src/models/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ Copyright (c) 2023 Uber Technologies, Inc.
limitations under the License.
*/

use std::collections::HashMap;

use derive_builder::Builder;
use getset::Getters;
use log::trace;
Expand Down Expand Up @@ -73,7 +75,7 @@ impl SourceCodeUnit {
) {
// Generate the scope query for the specific context by substituting the
// the tags with code snippets appropriately in the `generator` query.
return m.scope().instantiate(p_match.matches());
return m.scope().instantiate(p_match.matches(), &HashMap::new());
}
}
if let Some(parent) = changed_node.parent() {
Expand Down
14 changes: 7 additions & 7 deletions src/models/unit_tests/rule_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn test_rule_try_instantiate_positive() {
(String::from("variable_name"), String::from("foobar")),
(String::from("@a.lhs"), String::from("something")), // Should not substitute, since it `a.lhs` is not in `rule.holes`
]);
let instantiated_rule = InstantiatedRule::new(&rule, &substitutions);
let instantiated_rule = InstantiatedRule::new(&rule, &substitutions, &HashMap::new());
assert!(eq_without_whitespace(
instantiated_rule.query().get_query().as_str(),
"(((assignment_expression left: (_) @a.lhs right: (_) @a.rhs) @abc) (#eq? @a.lhs \"foobar\"))"
Expand All @@ -69,7 +69,7 @@ fn test_rule_try_instantiate_negative() {
let substitutions: HashMap<String, String> = HashMap::from([
(String::from("@a.lhs"), String::from("something")), // Should not substitute, since it `a.lhs` is not in `rule.holes`
]);
let _ = InstantiatedRule::new(&rule, &substitutions);
let _ = InstantiatedRule::new(&rule, &substitutions, &HashMap::new());
}

/// Positive tests for `rule.get_edit` method for given rule and input source code.
Expand Down Expand Up @@ -100,7 +100,7 @@ fn test_get_edit_positive_recursive() {
}
]
};
let rule = InstantiatedRule::new(&_rule, &HashMap::new());
let rule = InstantiatedRule::new(&_rule, &HashMap::new(),&HashMap::new());
let source_code = "class Test {
public void foobar(){
boolean isFlagTreated = true;
Expand Down Expand Up @@ -170,7 +170,7 @@ fn test_get_edit_negative_recursive() {
}
}";

let rule = InstantiatedRule::new(&_rule, &HashMap::new());
let rule = InstantiatedRule::new(&_rule, &HashMap::new(), &HashMap::new());
let mut rule_store = RuleStore::default();

let args = PiranhaArgumentsBuilder::default()
Expand Down Expand Up @@ -207,7 +207,7 @@ fn test_get_edit_for_context_positive() {
replace_node = "binary_expression",
replace = ""
};
let rule = InstantiatedRule::new(&_rule, &HashMap::new());
let rule = InstantiatedRule::new(&_rule, &HashMap::new(), &HashMap::new());

let source_code = "class A {
boolean f = something && true;
Expand Down Expand Up @@ -247,7 +247,7 @@ fn test_get_edit_for_context_negative() {
replace_node = "binary_expression",
replace = ""
};
let rule = InstantiatedRule::new(&_rule, &HashMap::new());
let rule = InstantiatedRule::new(&_rule, &HashMap::new(), &HashMap::new());
let source_code = "class A {
boolean f = true;
boolean x = something && true;
Expand Down Expand Up @@ -290,7 +290,7 @@ fn run_test_satisfies_filters_not_enclosing_node(
replace= "",
filters= [filter,]
};
let rule = InstantiatedRule::new(&_rule, &HashMap::new());
let rule = InstantiatedRule::new(&_rule, &HashMap::new(), &HashMap::new());
let source_code = "class Test {
public void foobar(){
if (isFlagTreated) {
Expand Down
Loading