Skip to content

Commit

Permalink
Use new configuration value for size of input files (#221)
Browse files Browse the repository at this point in the history
* Use new configuration value for size of input files

* cargo fmt
  • Loading branch information
alexevanczuk authored Dec 20, 2024
1 parent 4b92e79 commit 44a1770
Show file tree
Hide file tree
Showing 14 changed files with 73 additions and 36 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

[package]
name = "pks"
version = "0.2.17"
version = "0.2.18"
edition = "2021"
description = "Welcome! Please see https://github.com/alexevanczuk/packs for more information!"
license = "MIT"
Expand Down
3 changes: 2 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ mod test_util {
fixture_name: &str,
) -> anyhow::Result<Box<dyn ConstantResolver>> {
let absolute_root = get_absolute_root(fixture_name);
let configuration = configuration::get(&absolute_root)?;
let configuration = configuration::get(&absolute_root, &10)?;

Ok(get_zeitwerk_constant_resolver(
&configuration.pack_set,
Expand Down Expand Up @@ -85,6 +85,7 @@ mod test_util {
&default_absolute_root,
RawConfiguration::default(),
walk_directory_result,
&0,
)
.unwrap() // TODO: potentially convert `default` to `new` and return a Result
}
Expand Down
13 changes: 10 additions & 3 deletions src/packs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ pub fn add_dependency(
// (which takes ownership over the previous one).
// For now, we simply refetch the entire configuration for simplicity,
// since we don't mind the slowdown for this CLI command.
let new_configuration = configuration::get(&configuration.absolute_root)?;
let new_configuration = configuration::get(
&configuration.absolute_root,
&configuration.input_files_count,
)?;
let validation_result = packs::validate(&new_configuration);
if validation_result.is_err() {
println!("Added `{}` as a dependency to `{}`!", to, from);
Expand All @@ -238,9 +241,12 @@ pub fn validate(configuration: &Configuration) -> anyhow::Result<()> {
checker::validate_all(configuration)
}

pub fn configuration(project_root: PathBuf) -> anyhow::Result<Configuration> {
pub fn configuration(
project_root: PathBuf,
input_files_count: &usize,
) -> anyhow::Result<Configuration> {
let absolute_root = project_root.canonicalize()?;
configuration::get(&absolute_root)
configuration::get(&absolute_root, input_files_count)
}

pub fn check_unnecessary_dependencies(
Expand Down Expand Up @@ -439,6 +445,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&10,
)
.unwrap();
let absolute_file_path = configuration
Expand Down
2 changes: 1 addition & 1 deletion src/packs/caching/per_file_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ mod tests {

fn teardown() {
packs::delete_cache(
configuration::get(&PathBuf::from("tests/fixtures/simple_app"))
configuration::get(&PathBuf::from("tests/fixtures/simple_app"), &1)
.unwrap(),
);
}
Expand Down
3 changes: 3 additions & 0 deletions src/packs/checker/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&1,
)
.unwrap();

Expand All @@ -340,6 +341,7 @@ packs/foo, packs/bar",
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&1,
)
.unwrap();

Expand All @@ -358,6 +360,7 @@ packs/foo, packs/bar",
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&1,
)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions src/packs/checker/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&1,
)
.unwrap();
let checker = Checker {
Expand Down
6 changes: 3 additions & 3 deletions src/packs/checker/privacy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,19 @@ impl CheckerInterface for Checker {
let absolute_file =
configuration.absolute_root.join(relative_file);

// if configuration.included_files is less than 100, we're just going to individually
// if configuration.input_files_count is less than 100, we're just going to individually
// take the contents of the absolute file and call extract_sigils_from_contents on it to get the sigils
// and then check if a "public" sigil is contained. manual_read_of_defining_file_contains_sigil

let manual_read_of_defining_file_contains_sigil =
if configuration.included_files.len() < 100 {
if configuration.input_files_count < 100 {
if let Ok(contents) =
std::fs::read_to_string(&absolute_file)
{
let sigils =
ruby::parse_utils::extract_sigils_from_contents(
&contents,
);

sigils.iter().any(|sigil| sigil.name == "public")
} else {
false
Expand Down
5 changes: 4 additions & 1 deletion src/packs/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ pub fn run() -> anyhow::Result<()> {
packs::init(&absolute_root, use_packwerk)?
}

let mut configuration = packs::configuration::get(&absolute_root)?;
// Input filesize TBD
let mut configuration = packs::configuration::get(&absolute_root, &0)?;

if args.print_files {
configuration.print_files = true;
Expand Down Expand Up @@ -266,6 +267,7 @@ pub fn run() -> anyhow::Result<()> {
} => {
configuration.ignore_recorded_violations =
ignore_recorded_violations;
configuration.input_files_count = files.len();
packs::check(&configuration, files)
}
Command::CheckContents {
Expand All @@ -277,6 +279,7 @@ pub fn run() -> anyhow::Result<()> {

let absolute_path = get_absolute_path(file.clone(), &configuration);
configuration.stdin_file_path = Some(absolute_path);
configuration.input_files_count = 1;
packs::check(&configuration, vec![file])
}
Command::Update => packs::update(&configuration),
Expand Down
33 changes: 24 additions & 9 deletions src/packs/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use walk_directory::walk_directory;

pub struct Configuration {
pub included_files: HashSet<PathBuf>,
pub input_files_count: usize, // Helpful for optimizations in privacy chcker
pub absolute_root: PathBuf,
pub cache_enabled: bool,
pub cache_directory: PathBuf,
Expand Down Expand Up @@ -93,20 +94,29 @@ impl Configuration {
}
}

pub(crate) fn get(absolute_root: &Path) -> anyhow::Result<Configuration> {
pub(crate) fn get(
absolute_root: &Path,
input_files_count: &usize,
) -> anyhow::Result<Configuration> {
debug!("Beginning to build configuration");

let raw_config = raw_configuration::get(absolute_root)?;
let walk_directory_result =
walk_directory(absolute_root.to_path_buf(), &raw_config)?;

from_raw(absolute_root, raw_config, walk_directory_result)
from_raw(
absolute_root,
raw_config,
walk_directory_result,
input_files_count,
)
}

pub(crate) fn from_raw(
absolute_root: &Path,
raw_config: RawConfiguration,
walk_directory_result: WalkDirectoryResult,
input_files_count: &usize,
) -> anyhow::Result<Configuration> {
let WalkDirectoryResult {
included_files,
Expand Down Expand Up @@ -147,6 +157,7 @@ pub(crate) fn from_raw(

Ok(Configuration {
included_files,
input_files_count: input_files_count.to_owned(),
absolute_root,
cache_enabled,
cache_directory,
Expand Down Expand Up @@ -184,7 +195,7 @@ mod tests {
#[test]
fn default_options() {
let absolute_root = PathBuf::from("tests/fixtures/simple_app");
let actual = configuration::get(&absolute_root).unwrap();
let actual = configuration::get(&absolute_root, &0).unwrap();
assert_eq!(actual.absolute_root, absolute_root);

let expected_included_files = vec![
Expand Down Expand Up @@ -304,7 +315,7 @@ mod tests {
#[test]
fn filtered_absolute_paths_with_nonempty_input_paths() {
let absolute_root = PathBuf::from("tests/fixtures/simple_app");
let configuration = configuration::get(&absolute_root).unwrap();
let configuration = configuration::get(&absolute_root, &0).unwrap();
let actual_paths = configuration.intersect_files(vec![
String::from("packs/foo/app/services/foo.rb"),
String::from("scripts/my_script.rb"),
Expand All @@ -323,7 +334,7 @@ mod tests {
#[test]
fn filtered_absolute_paths_with_empty_input_paths() {
let absolute_root = PathBuf::from("tests/fixtures/simple_app");
let configuration = configuration::get(&absolute_root).unwrap();
let configuration = configuration::get(&absolute_root, &0).unwrap();
let actual_paths = configuration.intersect_files(vec![]);
let expected_paths = vec![
absolute_root.join("frontend/ui_helper.rb"),
Expand All @@ -344,7 +355,7 @@ mod tests {
#[test]
fn filtered_absolute_paths_with_directory_input_paths() {
let absolute_root = PathBuf::from("tests/fixtures/simple_app");
let configuration = configuration::get(&absolute_root).unwrap();
let configuration = configuration::get(&absolute_root, &0).unwrap();
let actual_paths =
configuration.intersect_files(vec![String::from("packs/bar")]);
let expected_paths = vec![
Expand Down Expand Up @@ -376,9 +387,13 @@ mod tests {
owning_package_yml_for_file: Default::default(),
};

let configuration =
configuration::from_raw(&absolute_root, raw, walk_directory_result)
.unwrap();
let configuration = configuration::from_raw(
&absolute_root,
raw,
walk_directory_result,
&0,
)
.unwrap();
let actual_associations = configuration.custom_associations;
let expected_paths = vec!["my_association".to_owned()];

Expand Down
2 changes: 2 additions & 0 deletions src/packs/dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&0,
)
.unwrap();

Expand All @@ -91,6 +92,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path")
.as_path(),
&0,
)
.unwrap();

Expand Down
7 changes: 4 additions & 3 deletions src/packs/monkey_patch_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,10 @@ These monkey patches redefine behavior in a pack within your app (as determined
</details>"
);

let mut configuration = configuration::get(&PathBuf::from(
"tests/fixtures/app_with_monkey_patches",
))
let mut configuration = configuration::get(
&PathBuf::from("tests/fixtures/app_with_monkey_patches"),
&0,
)
.unwrap();
configuration.experimental_parser = true;
let actual_message = expose_monkey_patches(
Expand Down
4 changes: 2 additions & 2 deletions src/packs/parsing/ruby/zeitwerk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ mod tests {

fn teardown() {
packs::delete_cache(
configuration::get(&PathBuf::from("tests/fixtures/simple_app"))
configuration::get(&PathBuf::from("tests/fixtures/simple_app"), &0)
.unwrap(),
);
}
Expand Down Expand Up @@ -487,7 +487,7 @@ mod tests {
.canonicalize()
.expect("Could not canonicalize path");

let configuration = configuration::get(absolute_root).unwrap();
let configuration = configuration::get(absolute_root, &0).unwrap();

let constant_resolver = get_zeitwerk_constant_resolver(
&configuration.pack_set,
Expand Down
7 changes: 4 additions & 3 deletions tests/add_constant_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ fn test_add_constant_dependencies() -> anyhow::Result<()> {
"Successfully updated 1 dependency for constant '::Bar::Tender'",
));

let config = packs::packs::configuration(PathBuf::from(
"tests/fixtures/app_with_missing_dependencies",
))
let config = packs::packs::configuration(
PathBuf::from("tests/fixtures/app_with_missing_dependencies"),
&0,
)
.unwrap();

let pack = config.pack_set.for_pack("packs/foo").unwrap();
Expand Down
21 changes: 12 additions & 9 deletions tests/add_dependency_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ fn test_add_dependency() -> Result<(), Box<dyn Error>> {
"Successfully added `packs/foo` as a dependency to `packs/baz`!",
));

let config = packs::packs::configuration(PathBuf::from(
"tests/fixtures/app_with_missing_dependency",
))
let config = packs::packs::configuration(
PathBuf::from("tests/fixtures/app_with_missing_dependency"),
&0,
)
.unwrap();

let pack = config.pack_set.for_pack("packs/baz").unwrap();
Expand Down Expand Up @@ -61,9 +62,10 @@ fn test_add_dependency_creating_cycle() -> Result<(), Box<dyn Error>> {
))
.stdout(predicate::str::contains("packs/foo, packs/bar"));

let config = packs::packs::configuration(PathBuf::from(
"tests/fixtures/app_with_missing_dependency",
))
let config = packs::packs::configuration(
PathBuf::from("tests/fixtures/app_with_missing_dependency"),
&0,
)
.unwrap();

let pack = config.pack_set.for_pack("packs/bar").unwrap();
Expand Down Expand Up @@ -92,9 +94,10 @@ fn test_add_dependency_unnecessarily() -> Result<(), Box<dyn Error>> {
"`packs/foo` already depends on `packs/bar`!",
));

let config = packs::packs::configuration(PathBuf::from(
"tests/fixtures/app_with_missing_dependency",
))
let config = packs::packs::configuration(
PathBuf::from("tests/fixtures/app_with_missing_dependency"),
&0,
)
.unwrap();

let pack = config.pack_set.for_pack("packs/foo").unwrap();
Expand Down

0 comments on commit 44a1770

Please sign in to comment.