diff --git a/schema/list-references-output.json b/schema/list-references-output.json index a0d166b2..6e7c71dc 100644 --- a/schema/list-references-output.json +++ b/schema/list-references-output.json @@ -43,7 +43,10 @@ "minimum": 0, "description": "Counted in bytes from 0. null when line is null." }, - "constant_name": { "type": "string" }, + "constant_name": { + "type": "string", + "description": "Fully qualified, as `::Foo::Bar`, though the source may write it relatively, as `Bar` inside `module Foo`. A reference that pks infers from an association, as `has_many :tacos` gives `::Taco`, is at the start of the call, where no constant is written. A `class_name:` option names the constant in a string." + }, "referencing_pack_name": { "type": "string" }, "defining_pack_name": { "type": "string" }, "violations": { diff --git a/src/packs/reference_lister.rs b/src/packs/reference_lister.rs index 1c9726c0..0e5f0c53 100644 --- a/src/packs/reference_lister.rs +++ b/src/packs/reference_lister.rs @@ -13,7 +13,7 @@ use rayon::prelude::{ParallelIterator, ParallelSlice}; use serde::Serialize; use super::{ - checker::get_checkers, + checker::{get_checkers, reference::Reference, CheckerInterface}, checker_configuration::CheckerType, reference_extractor::{between_packs, get_all_references, identity}, Configuration, @@ -63,31 +63,11 @@ pub(crate) fn list( between_packs(&all_references, &defining_packs, &referencing_packs); let checkers = get_checkers(configuration); - let recorded_violations = &configuration.pack_set.all_violations; let references = references - // A constant that a pack defines in several files yields a copy of the - // reference for each file, and their violations can differ, as when - // only one of the files is public. Like `check`, list each violation - // that any copy makes. + // With the experimental parser, a constant that a pack defines in + // several files yields a copy of the reference for each file .par_chunk_by(|a, b| identity(a) == identity(b)) .map(|copies| { - let mut violations = Vec::new(); - for checker in &checkers { - for reference in copies { - if let Some(violation) = - checker.check(reference, configuration)? - { - violations.push(JsonViolation { - strict: violation.strict, - recorded: recorded_violations - .contains(&violation.identifier), - violation_type: violation.identifier.violation_type, - }); - break; // One of each type, as `check` lists them - } - } - } - let reference = copies[0]; // pks keeps no source positions in ERB, and reports line 0 there let location = (reference.source_location.line > 0) @@ -101,8 +81,8 @@ pub(crate) fn list( defining_pack_name: reference .defining_pack_name .as_deref() - .expect("references that no pack defines are filtered out"), - violations, + .expect("between_packs drops constants no pack defines"), + violations: violations_of(copies, &checkers, configuration)?, }) }) .collect::>>()?; @@ -113,6 +93,32 @@ pub(crate) fn list( Ok(()) } +/// The violations that any copy of a reference makes, one of each type, as +/// `check` lists them. The copies' violations can differ, as when only one of +/// the files that define the constant is public. +fn violations_of( + copies: &[&Reference], + checkers: &[Box], + configuration: &Configuration, +) -> anyhow::Result> { + let recorded_violations = &configuration.pack_set.all_violations; + let mut violations = Vec::new(); + for checker in checkers { + for reference in copies { + if let Some(violation) = checker.check(reference, configuration)? { + violations.push(JsonViolation { + strict: violation.strict, + recorded: recorded_violations + .contains(&violation.identifier), + violation_type: violation.identifier.violation_type, + }); + break; + } + } + } + Ok(violations) +} + /// The packs given on the command line, which must exist. fn pack_names<'a>( configuration: &'a Configuration, diff --git a/tests/list_references_test.rs b/tests/list_references_test.rs index 3b33403b..0bee463b 100644 --- a/tests/list_references_test.rs +++ b/tests/list_references_test.rs @@ -492,6 +492,40 @@ fn test_list_references_at_the_start_of_a_file() -> Result<(), Box> { Ok(()) } +#[test] +fn test_list_references_from_associations() -> Result<(), Box> { + let fixture = common::Fixture::new("app_with_rails_relationships"); + // carrier.rb doesn't write `Census` or `Taco`, only `has_many :censuses` + // and `has_many :tacos`, so each reference is at the start of its call. + // notification.rb's polymorphic `belongs_to :event` infers none. + let reference = |line: u64, constant_name: &str| { + json!({ + "file": "packs/bar/app/models/carrier.rb", + "line": line, + "column": 2, + "constant_name": constant_name, + "referencing_pack_name": "packs/bar", + "defining_pack_name": "packs/baz", + "violations": [ + { + "violation_type": "privacy", + "strict": false, + "recorded": false + } + ] + }) + }; + + assert_eq!( + list_references(&fixture, &["list-references"]), + json!({ + "references": [reference(2, "::Census"), reference(3, "::Taco")] + }) + ); + + Ok(()) +} + #[test] fn test_list_references_lists_files_in_order() -> Result<(), Box> { let fixture = common::Fixture::new("simple_app");