CONTRIBUTING.md

@@ -47,31 +47,32 @@ The most important tools and commands in this environment are:
 ### Integration tests
 
 Integration tests are declared in [`./tests`](./tests) as subdirectories imitating Nixpkgs with these files:
-- `default.nix`:
-  Always contains
-  ```nix
-  import <test-nixpkgs> { root = ./.; }
-  ```
-  which makes
-  ```
-  nix-instantiate <subdir> --eval -A <attr> --arg overlays <overlays>
-  ```
-  work very similarly to the real Nixpkgs, just enough for the program to be able to test it.
-- `pkgs/by-name`:
-  The `pkgs/by-name` directory to check.
-
-- `pkgs/top-level/all-packages.nix` (optional):
-  Contains an overlay of the form
-  ```nix
-  self: super: {
-    # ...
-  }
-  ```
-  allowing the simulation of package overrides to the real [`pkgs/top-level/all-packages.nix`](https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix).
-  The default is an empty overlay.
+- `main`: A Nixpkgs root directory with:
+  - `default.nix`:
+    Always contains
+    ```nix
+    import <test-nixpkgs> { root = ./.; }
+    ```
+    which makes
+    ```
+    nix-instantiate <subdir> --eval -A <attr> --arg overlays <overlays>
+    ```
+    work very similarly to the real Nixpkgs, just enough for the program to be able to test it.
+  - `pkgs/by-name`:
+    The `pkgs/by-name` directory to check.
+
+  - `pkgs/top-level/all-packages.nix` (optional):
+    Contains an overlay of the form
+    ```nix
+    self: super: {
+      # ...
+    }
+    ```
+    allowing the simulation of package overrides to the real [`pkgs/top-level/all-packages.nix`](https://github.com/NixOS/nixpkgs/blob/master/pkgs/top-level/all-packages.nix).
+    The default is an empty overlay.
 
 - `base` (optional):
-  Contains another subdirectory imitating Nixpkgs with potentially any of the above structures.
+  Contains another Nixpkgs root directory with potentially any of the above structures.
   This is used to test [ratchet checks](./README.md#ratchet-checks).
 
 - `expected` (optional):

src/eval.rs

@@ -1,3 +1,4 @@
+use std::collections::BTreeMap;
 use std::path::{Path, PathBuf};
 use std::{env, fs, process};
 
@@ -156,7 +157,7 @@ pub fn check_values(
     nixpkgs_path: &Path,
     nix_file_store: &mut NixFileStore,
     package_names: &[String],
-) -> validation::Result<ratchet::Nixpkgs> {
+) -> validation::Result<BTreeMap<String, ratchet::Package>> {
     let work_dir = tempfile::Builder::new()
         .prefix("nixpkgs-vet")
         .tempdir()
@@ -255,10 +256,7 @@ pub fn check_values(
             .collect_vec()?,
     );
 
-    Ok(check_result.map(|elems| ratchet::Nixpkgs {
-        package_names: elems.iter().map(|(name, _)| name.to_owned()).collect(),
-        package_map: elems.into_iter().collect(),
-    }))
+    Ok(check_result.map(|elems| elems.into_iter().collect()))
 }
 
 /// Handle the evaluation result for an attribute in `pkgs/by-name`, making it a validation result.
@@ -373,7 +371,7 @@ fn by_name(
 
             // Independently report problems about whether it's a derivation and the callPackage
             // variant.
-            is_derivation_result.and(variant_result)
+            is_derivation_result.and_(variant_result)
         }
     };
     Ok(

src/files.rs

@@ -0,0 +1,106 @@
+use crate::problem::npv_169;
+use crate::problem::npv_170;
+use relative_path::RelativePath;
+use relative_path::RelativePathBuf;
+use rnix::ast::Expr::Str;
+use std::collections::BTreeMap;
+use std::path::Path;
+
+use crate::nix_file::NixFileStore;
+use crate::validation::ResultIteratorExt;
+use crate::validation::Validation::Success;
+use crate::{nix_file, ratchet, structure, validation};
+
+fn find_invalid_withs(syntax: SyntaxNode<NixLanguage>) -> Option<SyntaxNode<NixLanguage>> {
+    syntax
+        .descendants()
+        .filter(|node| node.kind() == rnix::SyntaxKind::NODE_WITH)
+        .filter(|node| {
+            node.descendants()
+                .map(|child| {
+                    if child == *node {
+                        return None;
+                    }
+                    let node_if_invalid = match child.kind() {
+                        SyntaxKind::NODE_WITH => Some(node),
+                        SyntaxKind::NODE_LET_IN => Some(node),
+                        SyntaxKind::NODE_ATTR_SET => Some(node),
+                        _ => None,
+                    };
+                    println!(
+                        "validate with={:?} subexpr={:?} invalid={:?}",
+                        node.to_string(),
+                        child.to_string(),
+                        node_if_invalid
+                    );
+                    node_if_invalid
+                })
+                .any(|cond| cond != None)
+        })
+        .take(1)
+        .last()
+}
+
+pub fn check_files(
+    nixpkgs_path: &Path,
+    nix_file_store: &mut NixFileStore,
+) -> validation::Result<BTreeMap<RelativePathBuf, ratchet::File>> {
+    process_nix_files(nixpkgs_path, nix_file_store, |nix_file| {
+        if let Some(open_scope_with_lib) = find_invalid_withs(nix_file.syntax_root) {
+            // TODO: what do I return
+            // return ratchet::RatchetState::Loose(
+            //     npv_169::TopLevelWithMayShadowVariablesAndBreakStaticChecks::new(
+            //         nix_file.relative_path,
+            //     )
+            //     .into(),
+            // );
+        }
+        Ok(Success(ratchet::File {}))
+    })
+}
+
+fn collect_nix_files(
+    base: &Path,
+    dir: &RelativePath,
+    files: &mut Vec<RelativePathBuf>,
+) -> anyhow::Result<()> {
+    for entry in structure::read_dir_sorted(&dir.to_path(base))? {
+        let mut relative_path = dir.to_relative_path_buf();
+        relative_path.push(entry.file_name().to_string_lossy().into_owned());
+
+        let absolute_path = entry.path();
+
+        if absolute_path.is_symlink() {
+            continue;
+        }
+        if absolute_path.is_dir() {
+            collect_nix_files(base, &relative_path, files)?
+        } else if absolute_path.extension().is_some_and(|x| x == "nix") {
+            files.push(relative_path)
+        }
+    }
+    Ok(())
+}
+
+fn process_nix_files<F: Fn(&nix_file::NixFile) -> validation::Result<ratchet::File>>(
+    nixpkgs_path: &Path,
+    nix_file_store: &mut NixFileStore,
+    f: F,
+) -> validation::Result<BTreeMap<RelativePathBuf, ratchet::File>> {
+    let files = {
+        let mut files = vec![];
+        collect_nix_files(nixpkgs_path, &RelativePathBuf::new(), &mut files)?;
+        files
+    };
+
+    let file_results: Vec<validation::Validation<(RelativePathBuf, ratchet::File)>> = files
+        .into_iter()
+        .map(|path| {
+            let nix_file = nix_file_store.get(&path.to_path(nixpkgs_path))?;
+            let val = f(nix_file)?.map(|file| (path, file));
+            Ok::<_, anyhow::Error>(val)
+        })
+        .collect_vec()?;
+
+    Ok(validation::sequence(file_results).map(|entries| entries.into_iter().collect()))
+}

src/main.rs

@@ -10,6 +10,7 @@
 // #![allow(clippy::missing_const_for_fn)]
 
 mod eval;
+mod files;
 mod location;
 mod nix_file;
 mod problem;
@@ -21,6 +22,7 @@ mod validation;
 
 use anyhow::Context as _;
 use clap::Parser;
+use std::collections::BTreeMap;
 use std::path::{Path, PathBuf};
 use std::process::ExitCode;
 use std::{panic, thread};
@@ -113,20 +115,30 @@ fn check_nixpkgs(nixpkgs_path: &Path) -> validation::Result<ratchet::Nixpkgs> {
         )
     })?;
 
-    if !nixpkgs_path.join(structure::BASE_SUBPATH).exists() {
-        // No pkgs/by-name directory, always valid
-        return Ok(Success(ratchet::Nixpkgs::default()));
-    }
-
     let mut nix_file_store = NixFileStore::default();
-    let structure = check_structure(&nixpkgs_path, &mut nix_file_store)?;
 
-    // Only if we could successfully parse the structure, we do the evaluation checks
-    let result = structure.result_map(|package_names| {
-        eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names.as_slice())
-    })?;
+    let package_result = {
+        if !nixpkgs_path.join(structure::BASE_SUBPATH).exists() {
+            // No pkgs/by-name directory, always valid
+            Success(BTreeMap::new())
+        } else {
+            let structure = check_structure(&nixpkgs_path, &mut nix_file_store)?;
+
+            // Only if we could successfully parse the structure, we do the evaluation checks
+            structure.result_map(|package_names| {
+                eval::check_values(&nixpkgs_path, &mut nix_file_store, package_names.as_slice())
+            })?
+        }
+    };
+
+    let file_result = files::check_files(&nixpkgs_path, &mut nix_file_store)?;
 
-    Ok(result)
+    Ok(
+        package_result.and(file_result, |packages, files| ratchet::Nixpkgs {
+            packages,
+            files,
+        }),
+    )
 }
 
 #[cfg(test)]
@@ -178,7 +190,7 @@ mod tests {
             return Ok(());
         }
 
-        let base = path.join(BASE_SUBPATH);
+        let base = path.join("main").join(BASE_SUBPATH);
 
         fs::create_dir_all(base.join("fo/foo"))?;
         fs::write(base.join("fo/foo/package.nix"), "{ someDrv }: someDrv")?;
@@ -237,7 +249,7 @@ mod tests {
             .build()
             .expect("valid regex");
 
-        let path = path.to_owned();
+        let main_path = path.join("main");
         let base_path = path.join("base");
         let base_nixpkgs = if base_path.exists() {
             base_path
@@ -251,7 +263,7 @@ mod tests {
         let nix_conf_dir = nix_conf_dir.path().as_os_str();
 
         let status = temp_env::with_var("NIX_CONF_DIR", Some(nix_conf_dir), || {
-            process(base_nixpkgs, &path)
+            process(base_nixpkgs, &main_path)
         });
 
         let actual_errors = format!("{status}\n");

src/ratchet.rs

@@ -2,7 +2,8 @@
 //!
 //! Each type has a `compare` method that validates the ratchet checks for that item.
 
-use std::collections::HashMap;
+use relative_path::RelativePath;
+use std::collections::BTreeMap;
 
 use relative_path::RelativePathBuf;
 
@@ -13,10 +14,9 @@ use crate::validation::{self, Validation, Validation::Success};
 /// The ratchet value for the entirety of Nixpkgs.
 #[derive(Default)]
 pub struct Nixpkgs {
-    /// Sorted list of packages in `package_map`
-    pub package_names: Vec<String>,
     /// The ratchet values for all packages
-    pub package_map: HashMap<String, Package>,
+    pub packages: BTreeMap<String, Package>,
+    pub files: BTreeMap<RelativePathBuf, File>,
 }
 
 impl Nixpkgs {
@@ -25,10 +25,13 @@ impl Nixpkgs {
         validation::sequence_(
             // We only loop over the current attributes,
             // we don't need to check ones that were removed
-            to.package_names.into_iter().map(|name| {
-                Package::compare(&name, from.package_map.get(&name), &to.package_map[&name])
-            }),
+            to.packages
+                .into_iter()
+                .map(|(name, pkg)| Package::compare(&name, from.packages.get(&name), &pkg)),
         )
+        .and_(validation::sequence_(to.files.into_iter().map(
+            |(name, file)| File::compare(&name, from.files.get(&name), &file),
+        )))
     }
 }
 
@@ -59,6 +62,19 @@ impl Package {
     }
 }
 
+pub struct File {}
+
+impl File {
+    /// Validates the ratchet checks for a top-level package
+    pub fn compare(
+        _name: &RelativePath,
+        _optional_from: Option<&Self>,
+        _to: &Self,
+    ) -> Validation<()> {
+        Success(())
+    }
+}
+
 /// The ratchet state of a generic ratchet check.
 pub enum RatchetState<Ratchet: ToProblem> {
     /// The ratchet is loose. It can be tightened more. In other words, this is the legacy state

src/structure.rs

@@ -96,7 +96,7 @@ pub fn check_structure(
                         .into()
                     });
 
-                let result = result.and(validation::sequence_(duplicate_results));
+                let result = result.and_(validation::sequence_(duplicate_results));
 
                 let package_results = entries
                     .into_iter()
@@ -111,7 +111,7 @@ pub fn check_structure(
                     })
                     .collect_vec()?;
 
-                result.and(validation::sequence(package_results))
+                result.and_(validation::sequence(package_results))
             })
         })
         .collect_vec()?;
@@ -147,7 +147,7 @@ fn check_package(
         };
 
         let correct_relative_package_dir = relative_dir_for_package(&package_name);
-        let result = result.and(if relative_package_dir != correct_relative_package_dir {
+        let result = result.and_(if relative_package_dir != correct_relative_package_dir {
             // Only show this error if we have a valid shard and package name.
             // If one of those is wrong, you should fix that first.
             if shard_name_valid && package_name_valid {
@@ -164,15 +164,15 @@ fn check_package(
         });
 
         let package_nix_path = package_path.join(PACKAGE_NIX_FILENAME);
-        let result = result.and(if !package_nix_path.exists() {
+        let result = result.and_(if !package_nix_path.exists() {
             npv_143::PackageNixMissing::new(package_name.clone()).into()
         } else if !package_nix_path.is_file() {
             npv_144::PackageNixIsNotFile::new(package_name.clone()).into()
         } else {
             Success(())
         });
 
-        let result = result.and(references::check_references(
+        let result = result.and_(references::check_references(
             nix_file_store,
             &relative_package_dir,
             &relative_package_dir.to_path(path),