Setting ViolationIdentifier strict in all checkers (#169)

* setting violation-identifier strict in all checkers

* recorded violation are not strict

src/packs/checker.rs

@@ -34,6 +34,7 @@ use super::reference_extractor::get_all_references;
 #[derive(PartialEq, Clone, Eq, Hash, Debug)]
 pub struct ViolationIdentifier {
     pub violation_type: String,
+    pub strict: bool,
     pub file: String,
     pub constant_name: String,
     pub referencing_pack_name: String,

src/packs/checker/architecture.rs

@@ -171,6 +171,7 @@ impl CheckerInterface for Checker {
                 let file = reference.relative_referencing_file.clone();
                 let identifier = ViolationIdentifier {
                     violation_type,
+                    strict: referencing_pack.enforce_architecture().is_strict(),
                     file,
                     constant_name: reference.constant_name.clone(),
                     referencing_pack_name: referencing_pack_name.clone(),
@@ -250,7 +251,7 @@ mod tests {
             },
             expected_violation: Some(build_expected_violation(
                 "packs/foo/app/services/foo.rb:3:1\nArchitecture violation: `::Bar` belongs to `packs/bar` (whose layer is `product`) cannot be accessed from `packs/foo` (whose layer is `utilities`)".to_string(), 
-                "architecture".to_string())),
+                "architecture".to_string(), false)),
             ..Default::default()
         };
         test_check(&checker_with_layers(), &mut test_checker)
@@ -275,7 +276,32 @@ mod tests {
             },
             expected_violation: Some(build_expected_violation(
                 "packs/foo/app/services/foo.rb:3:1\nArchitecture violation: `::Bar` belongs to `packs/bar` (whose layer is `product`) cannot be accessed from `packs/foo` (whose layer is `utilities`)".to_string(), 
-                "architecture".to_string())),
+                "architecture".to_string(), false)),
+            ..Default::default()
+        };
+        test_check(&checker_with_layers(), &mut test_checker)
+    }
+
+    #[test]
+    fn reference_is_a_strict_violation() -> anyhow::Result<()> {
+        let mut test_checker = TestChecker {
+            reference: None,
+            configuration: None,
+            referenced_constant_name: Some(String::from("::Bar")),
+            defining_pack: Some(Pack {
+                name: "packs/bar".to_owned(),
+               layer: Some("product".to_string()),
+                ..default_defining_pack()
+            }),
+            referencing_pack: Pack {
+                name: "packs/foo".to_owned(),
+                enforce_architecture: Some(CheckerSetting::Strict),
+                layer: Some("utilities".to_string()),
+                ..default_referencing_pack()
+            },
+            expected_violation: Some(build_expected_violation(
+                "packs/foo/app/services/foo.rb:3:1\nArchitecture violation: `::Bar` belongs to `packs/bar` (whose layer is `product`) cannot be accessed from `packs/foo` (whose layer is `utilities`)".to_string(), 
+                "architecture".to_string(), true)),
             ..Default::default()
         };
         test_check(&checker_with_layers(), &mut test_checker)

src/packs/checker/common_test.rs

@@ -28,25 +28,29 @@ pub mod tests {
     pub fn build_expected_violation(
         message: String,
         violation_type: String,
+        strict: bool,
     ) -> Violation {
         build_expected_violation_with_constant(
             message,
             violation_type,
+            strict,
             String::from("::Bar"),
         )
     }
 
     pub fn build_expected_violation_with_constant(
         message: String,
         violation_type: String,
+        strict: bool,
         constant_name: String,
     ) -> Violation {
         Violation {
             message,
             identifier: ViolationIdentifier {
                 violation_type,
+                strict,
                 file: String::from("packs/foo/app/services/foo.rb"),
-                constant_name: constant_name,
+                constant_name,
                 referencing_pack_name: String::from("packs/foo"),
                 defining_pack_name: String::from("packs/bar"),
             },

src/packs/checker/dependency.rs

@@ -165,6 +165,7 @@ impl CheckerInterface for Checker {
             let file = reference.relative_referencing_file.clone();
             let identifier = ViolationIdentifier {
                 violation_type,
+                strict: referencing_pack.enforce_dependencies().is_strict(),
                 file,
                 constant_name: reference.constant_name.clone(),
                 referencing_pack_name: referencing_pack_name.clone(),
@@ -248,7 +249,29 @@ mod tests {
                 ..default_referencing_pack()},
             expected_violation: Some(build_expected_violation(
                 "packs/foo/app/services/foo.rb:3:1\nDependency violation: `::Bar` belongs to `packs/bar`, but `packs/foo/package.yml` does not specify a dependency on `packs/bar`.".to_string(),
-                "dependency".to_string())),
+                "dependency".to_string(), false)),
+            ..Default::default()
+        };
+        test_check(&Checker {}, &mut test_checker)
+    }
+
+    #[test]
+    fn test_with_strict_violation() -> anyhow::Result<()> {
+        let mut test_checker = TestChecker {
+            reference: None,
+            configuration: None,
+            referenced_constant_name: Some(String::from("::Bar")),
+            defining_pack: Some(Pack {
+                name: "packs/bar".to_owned(),
+                ..default_defining_pack()
+            }),
+            referencing_pack: Pack{
+                relative_path: PathBuf::from("packs/foo"),
+                enforce_dependencies: Some(CheckerSetting::Strict),
+                ..default_referencing_pack()},
+            expected_violation: Some(build_expected_violation(
+                "packs/foo/app/services/foo.rb:3:1\nDependency violation: `::Bar` belongs to `packs/bar`, but `packs/foo/package.yml` does not specify a dependency on `packs/bar`.".to_string(),
+                "dependency".to_string(), true)),
             ..Default::default()
         };
         test_check(&Checker {}, &mut test_checker)

src/packs/checker/folder_visibility.rs

@@ -35,6 +35,7 @@ impl CheckerInterface for Checker {
             );
             let identifier = ViolationIdentifier {
                 violation_type: self.violation_type(),
+                strict: defining_pack.enforce_folder_visibility().is_strict(),
                 file: reference.relative_referencing_file.clone(),
                 constant_name: reference.constant_name.clone(),
                 referencing_pack_name: referencing_pack.name.clone(),
@@ -117,7 +118,28 @@ mod tests {
                 ..default_referencing_pack()},
             expected_violation: Some(build_expected_violation(
                 "packs/foo/app/services/foo.rb:3:1\nFolder Visibility violation: `::Bar` belongs to `packs/bar`, which is not visible to `packs/foo` as it is not a sibling pack or parent pack.".to_string(),
-                "folder_visibility".to_string())),
+                "folder_visibility".to_string(), false)),
+            ..Default::default()
+        };
+        test_check(&Checker {}, &mut test_checker)
+    }
+    #[test]
+    fn test_with_strict_violation() -> anyhow::Result<()> {
+        let mut test_checker = TestChecker {
+            reference: None,
+            configuration: None,
+            referenced_constant_name: Some(String::from("::Bar")),
+            defining_pack: Some(Pack {
+                name: "packs/bar".to_owned(),
+                enforce_folder_visibility: Some(CheckerSetting::Strict),
+                ..default_defining_pack()
+            }),
+            referencing_pack: Pack{
+                relative_path: PathBuf::from("packs/foo"),
+                ..default_referencing_pack()},
+            expected_violation: Some(build_expected_violation(
+                "packs/foo/app/services/foo.rb:3:1\nFolder Visibility violation: `::Bar` belongs to `packs/bar`, which is not visible to `packs/foo` as it is not a sibling pack or parent pack.".to_string(),
+                "folder_visibility".to_string(), true)),
             ..Default::default()
         };
         test_check(&Checker {}, &mut test_checker)

src/packs/checker/privacy.rs

@@ -102,6 +102,7 @@ impl CheckerInterface for Checker {
         let file = reference.relative_referencing_file.clone();
         let identifier = ViolationIdentifier {
             violation_type,
+            strict: defining_pack.enforce_privacy().is_strict(),
             file,
             constant_name: reference.constant_name.clone(),
             referencing_pack_name: referencing_pack_name.clone(),
@@ -201,7 +202,29 @@ mod tests {
             referencing_pack: default_referencing_pack(),
             expected_violation: Some(build_expected_violation(
                 String::from("packs/foo/app/services/foo.rb:3:1\nPrivacy violation: `::Bar` is private to `packs/bar`, but referenced from `packs/foo`"),
-                String::from("privacy"),
+                String::from("privacy"), false,
+            )),
+            ..Default::default()
+        };
+        test_check(&Checker {}, &mut test_checker)
+    }
+
+    #[test]
+    fn test_with_strict_privacy_violation() -> anyhow::Result<()> {
+        let mut test_checker = TestChecker {
+            reference: None,
+            configuration: None,
+            referenced_constant_name: Some(String::from("::Bar")),
+            defining_pack: Some(Pack {
+                name: "packs/bar".to_owned(),
+                enforce_privacy: Some(CheckerSetting::Strict),
+                ignored_private_constants: HashSet::from([String::from("::Taco")]),
+                ..default_defining_pack()
+            }),
+            referencing_pack: default_referencing_pack(),
+            expected_violation: Some(build_expected_violation(
+                String::from("packs/foo/app/services/foo.rb:3:1\nPrivacy violation: `::Bar` is private to `packs/bar`, but referenced from `packs/foo`"),
+                String::from("privacy"), true,
             )),
             ..Default::default()
         };
@@ -376,7 +399,7 @@ mod tests {
             referencing_pack: default_referencing_pack(),
             expected_violation: Some(build_expected_violation_with_constant(
                 String::from("packs/foo/app/services/foo.rb:3:1\nPrivacy violation: `::Bar::BarChild` is private to `packs/bar`, but referenced from `packs/foo`"),
-                String::from("privacy"),
+                String::from("privacy"), false,
                 String::from("::Bar::BarChild")
             )),
             ..Default::default()

src/packs/checker/visibility.rs

@@ -64,6 +64,7 @@ impl CheckerInterface for Checker {
         let file = reference.relative_referencing_file.clone();
         let identifier = ViolationIdentifier {
             violation_type,
+            strict: defining_pack.enforce_visibility().is_strict(),
             file,
             constant_name: reference.constant_name.clone(),
             referencing_pack_name: referencing_pack_name.clone(),
@@ -144,7 +145,29 @@ mod tests {
                 ..default_referencing_pack()},
             expected_violation: Some(build_expected_violation(
                 "packs/foo/app/services/foo.rb:3:1\nVisibility violation: `::Bar` belongs to `packs/bar`, which is not visible to `packs/foo`".to_string(),
-                "visibility".to_string())),
+                "visibility".to_string(), false)),
+            ..Default::default()
+        };
+        test_check(&Checker {}, &mut test_checker)
+    }
+
+    #[test]
+    fn test_with_strict_violation() -> anyhow::Result<()> {
+        let mut test_checker = TestChecker {
+            reference: None,
+            configuration: None,
+            referenced_constant_name: Some(String::from("::Bar")),
+            defining_pack: Some(Pack {
+                name: "packs/bar".to_owned(),
+                enforce_visibility: Some(CheckerSetting::Strict),
+                ..default_defining_pack()
+            }),
+            referencing_pack: Pack{
+                relative_path: PathBuf::from("packs/foo"),
+                ..default_referencing_pack()},
+            expected_violation: Some(build_expected_violation(
+                "packs/foo/app/services/foo.rb:3:1\nVisibility violation: `::Bar` belongs to `packs/bar`, which is not visible to `packs/foo`".to_string(),
+                "visibility".to_string(), true)),
             ..Default::default()
         };
         test_check(&Checker {}, &mut test_checker)

src/packs/pack.rs

@@ -151,6 +151,7 @@ impl Pack {
                     for file in &violation_group.files {
                         let identifier = ViolationIdentifier {
                             violation_type: violation_type.clone(),
+                            strict: false,
                             file: file.clone(),
                             constant_name: constant_name.clone(),
                             referencing_pack_name: self.name.clone(),
@@ -623,4 +624,39 @@ owner: Foobar
             vec![root.join("app/company_data"), root.join("app/services")];
         assert_eq!(expected, actual)
     }
+
+    #[test]
+    fn test_all_recorded_violations() -> anyhow::Result<()> {
+        let root = test_util::get_absolute_root(
+            "tests/fixtures/contains_package_todo",
+        );
+        let pack = Pack::from_path(
+            root.join("packs/foo/package.yml").as_path(),
+            root.as_path(),
+        )?;
+
+        let mut actual = pack.all_violations();
+        actual.sort_by(|a, b| a.file.cmp(&b.file));
+
+        let expected = vec![
+            ViolationIdentifier {
+                violation_type: "dependency".to_string(),
+                strict: false,
+                file: "packs/foo/app/services/foo.rb".to_string(),
+                constant_name: "::Bar".to_string(),
+                referencing_pack_name: "packs/foo".to_string(),
+                defining_pack_name: "packs/bar".to_string(),
+            },
+            ViolationIdentifier {
+                violation_type: "dependency".to_string(),
+                strict: false,
+                file: "packs/foo/app/services/other_foo.rb".to_string(),
+                constant_name: "::Bar".to_string(),
+                referencing_pack_name: "packs/foo".to_string(),
+                defining_pack_name: "packs/bar".to_string(),
+            },
+        ];
+        assert_eq!(expected, actual);
+        Ok(())
+    }
 }