fix(charmlint): docs and licence rules are monorepo-aware (#65)

CHANGELOG.md

@@ -5,6 +5,19 @@ All notable changes to Cantrip are documented here. This project is pre-1.0; onl
 ## Unreleased
 
 ### Fixed
+- **charmlint's docs and licence rules are monorepo-aware.**  DOC002-DOC005
+  (installation / configuration / usage / troubleshooting docs) looked only in
+  ``<charm>/docs``, and STR001 looked only for ``<charm>/LICENSE``.  A monorepo
+  keeps its charms under ``charms/<name>/`` with one shared ``docs/`` tree and
+  one licence at the repository root, so every charm in such a repository was
+  reported as undocumented and unlicensed — five false positives per charm.
+  ``CharmContext`` now records the enclosing ``repo_root`` (nearest ancestor
+  with a ``.git`` entry, so linked worktrees and submodules count) and exposes
+  ``search_roots()``; those rules search the charm directory first and fall
+  back to the repository root.  Charm-local assets still win, and a charm
+  outside a repository behaves exactly as before.  Fixed in both the Python
+  and Rust implementations.  Closes
+  [#65](https://github.com/tonyandrewmeyer/cantrip/issues/65).
 - **``subtask: true`` on a ``primary`` custom command no longer fails.**
   ``docs/src/howto-custom-commands.md`` documents ``subtask`` as "force
   work-queue dispatch even when ``agent`` is ``primary``", but the

src/charmlint-rs/src/context.rs

@@ -112,6 +112,22 @@ fn value_to_map(v: &Value) -> BTreeMap<String, Value> {
     }
 }
 
+/// Find the repository root enclosing `charm_dir`, if any.
+///
+/// The nearest ancestor holding a `.git` entry wins.  Existence rather than
+/// directory-ness is tested because `.git` is a plain file in linked worktrees
+/// and submodules, which are repository roots just the same.
+fn find_repo_root(charm_dir: &Path) -> Option<PathBuf> {
+    let mut candidate = Some(charm_dir);
+    while let Some(dir) = candidate {
+        if dir.join(".git").exists() {
+            return Some(dir.to_path_buf());
+        }
+        candidate = dir.parent();
+    }
+    None
+}
+
 pub fn build_context(charm_dir: &Path) -> CharmContext {
     let charm_dir = charm_dir
         .canonicalize()
@@ -174,6 +190,8 @@ pub fn build_context(charm_dir: &Path) -> CharmContext {
 
     let (has_tests_unit, has_tests_integration) = check_tests(&charm_dir);
 
+    let repo_root = find_repo_root(&charm_dir);
+
     CharmContext {
         charm_dir,
         metadata,
@@ -184,6 +202,7 @@ pub fn build_context(charm_dir: &Path) -> CharmContext {
         readme_content,
         has_tests_unit,
         has_tests_integration,
+        repo_root,
     }
 }
 

src/charmlint-rs/src/models.rs

@@ -86,6 +86,24 @@ pub struct CharmContext {
     pub readme_content: String,
     pub has_tests_unit: bool,
     pub has_tests_integration: bool,
+    /// Repository root enclosing `charm_dir`, or None when the charm is not
+    /// inside a repository.  Monorepos share repository-level assets (a single
+    /// `docs/` tree, one licence) between charms, so rules that look for such
+    /// assets must search here too.
+    pub repo_root: Option<PathBuf>,
+}
+
+impl CharmContext {
+    /// The directories to search for repository-level assets: the charm's own
+    /// directory first, then the repository root when it is a different
+    /// directory.  Charm-local assets therefore still win, and a charm outside
+    /// a repository behaves exactly as before.
+    pub fn search_roots(&self) -> Vec<&Path> {
+        match &self.repo_root {
+            Some(root) if root != &self.charm_dir => vec![&self.charm_dir, root],
+            _ => vec![&self.charm_dir],
+        }
+    }
 }
 
 /// Aggregated lint results.

src/charmlint-rs/src/rules.rs

@@ -629,8 +629,13 @@ fn check_security(ctx: &CharmContext) -> Vec<Diagnostic> {
 fn check_structure(ctx: &CharmContext) -> Vec<Diagnostic> {
     let mut diagnostics = Vec::new();
 
-    // STR001: no licence.
-    if !ctx.charm_dir.join("LICENSE").exists() && !ctx.charm_dir.join("LICENCE").exists() {
+    // STR001: no licence.  A monorepo normally carries one licence at the
+    // repository root rather than a copy in every charm directory.
+    let has_licence = ctx
+        .search_roots()
+        .iter()
+        .any(|root| root.join("LICENSE").exists() || root.join("LICENCE").exists());
+    if !has_licence {
         diagnostics.push(diag(
             "STR001",
             Severity::Info,
@@ -718,21 +723,27 @@ fn check_documentation(ctx: &CharmContext) -> Vec<Diagnostic> {
     diagnostics
 }
 
+/// Whether `keyword` appears in the README or in any `docs/` tree.
+///
+/// Monorepos commonly keep one `docs/` tree at the repository root, shared by
+/// every charm under `charms/<name>/`, so the repository root is searched as
+/// well as the charm's own directory.
 fn check_doc_topic(ctx: &CharmContext, keyword: &str) -> bool {
     if ctx.readme_content.to_lowercase().contains(keyword) {
         return true;
     }
-    let docs_dir = ctx.charm_dir.join("docs");
-    if docs_dir.is_dir() {
-        for entry in WalkDir::new(&docs_dir).follow_links(true) {
-            if let Ok(e) = entry {
-                if e.file_type().is_file()
-                    && e.path().extension().map_or(false, |ext| ext == "md")
-                {
-                    if let Ok(content) = std::fs::read_to_string(e.path()) {
-                        if content.to_lowercase().contains(keyword) {
-                            return true;
-                        }
+    for root in ctx.search_roots() {
+        let docs_dir = root.join("docs");
+        if !docs_dir.is_dir() {
+            continue;
+        }
+        for entry in WalkDir::new(&docs_dir).follow_links(true).into_iter().flatten() {
+            if entry.file_type().is_file()
+                && entry.path().extension().map_or(false, |ext| ext == "md")
+            {
+                if let Ok(content) = std::fs::read_to_string(entry.path()) {
+                    if content.to_lowercase().contains(keyword) {
+                        return true;
                     }
                 }
             }
@@ -1609,4 +1620,148 @@ mod tests {
         let set: HashSet<&str> = ["summary"].into_iter().collect();
         assert!(suggest_closest("completely-different", &set).is_none());
     }
+
+    // ── Monorepo awareness ──────────────────────────────────────
+
+    /// Build a monorepo fixture holding one charm under `charms/test-charm/`.
+    ///
+    /// The charm README mentions none of the DOC002-DOC005 topics, so those
+    /// rules can only pass via a repository-level `docs/` tree.
+    fn monorepo() -> tempfile::TempDir {
+        let dir = tempfile::tempdir().unwrap();
+        std::fs::create_dir(dir.path().join(".git")).unwrap();
+        let charm_dir = dir.path().join("charms/test-charm");
+        std::fs::create_dir_all(charm_dir.join("src")).unwrap();
+        write(&charm_dir.join("charmcraft.yaml"), "name: test-charm\n");
+        write(&charm_dir.join("README.md"), "# test-charm\n\nA charm.\n");
+        dir
+    }
+
+    fn write_shared_docs(repo_root: &std::path::Path, topics: &[&str]) {
+        for topic in topics {
+            write(
+                &repo_root.join("docs").join(format!("{topic}.md")),
+                &format!("# {topic}\n\nHow to {topic} the charms.\n"),
+            );
+        }
+    }
+
+    #[test]
+    fn shared_docs_tree_satisfies_doc_topics() {
+        let dir = monorepo();
+        write_shared_docs(
+            dir.path(),
+            &["installation", "configuration", "usage", "troubleshooting"],
+        );
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        for unwanted in ["DOC002", "DOC003", "DOC004", "DOC005"] {
+            assert!(!ids.contains(unwanted), "unexpected {unwanted}");
+        }
+    }
+
+    #[test]
+    fn shared_docs_tree_only_covers_the_topics_it_documents() {
+        let dir = monorepo();
+        write_shared_docs(dir.path(), &["installation"]);
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        assert!(!ids.contains("DOC002"));
+        for wanted in ["DOC003", "DOC004", "DOC005"] {
+            assert!(ids.contains(wanted), "missing {wanted}");
+        }
+    }
+
+    #[test]
+    fn repo_without_shared_docs_still_flags_topics() {
+        let dir = monorepo();
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        for wanted in ["DOC002", "DOC003", "DOC004", "DOC005"] {
+            assert!(ids.contains(wanted), "missing {wanted}");
+        }
+    }
+
+    #[test]
+    fn charm_outside_a_repository_still_flags_topics() {
+        // Regression guard — the fallback must not fire without a repository.
+        let dir = tempfile::tempdir().unwrap();
+        let charm_dir = dir.path().join("test-charm");
+        std::fs::create_dir_all(charm_dir.join("src")).unwrap();
+        write(&charm_dir.join("charmcraft.yaml"), "name: test-charm\n");
+        write_shared_docs(dir.path(), &["installation", "troubleshooting"]);
+        let ids = rule_ids(&run_rules(&charm_dir));
+        for wanted in ["DOC002", "DOC005"] {
+            assert!(ids.contains(wanted), "missing {wanted}");
+        }
+    }
+
+    #[test]
+    fn charm_local_docs_still_win() {
+        let dir = monorepo();
+        let charm_dir = dir.path().join("charms/test-charm");
+        write_shared_docs(&charm_dir, &["installation"]);
+        let ids = rule_ids(&run_rules(&charm_dir));
+        assert!(!ids.contains("DOC002"));
+    }
+
+    #[test]
+    fn shared_licence_satisfies_str001() {
+        let dir = monorepo();
+        write(&dir.path().join("LICENSE"), "Apache-2.0");
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        assert!(!ids.contains("STR001"));
+    }
+
+    #[test]
+    fn shared_british_spelling_licence_satisfies_str001() {
+        let dir = monorepo();
+        write(&dir.path().join("LICENCE"), "Apache-2.0");
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        assert!(!ids.contains("STR001"));
+    }
+
+    #[test]
+    fn repo_without_licence_still_flags_str001() {
+        let dir = monorepo();
+        let ids = rule_ids(&run_rules(&dir.path().join("charms/test-charm")));
+        assert!(ids.contains("STR001"));
+    }
+
+    #[test]
+    fn git_file_marks_a_worktree_root() {
+        // `.git` is a file, not a directory, in worktrees and submodules.
+        let dir = tempfile::tempdir().unwrap();
+        let charm_dir = dir.path().join("charms/test-charm");
+        std::fs::create_dir_all(charm_dir.join("src")).unwrap();
+        write(&charm_dir.join("charmcraft.yaml"), "name: test-charm\n");
+        write(&dir.path().join(".git"), "gitdir: /elsewhere/.git/worktrees/wt\n");
+        write(&dir.path().join("LICENSE"), "Apache-2.0");
+        let ids = rule_ids(&run_rules(&charm_dir));
+        assert!(!ids.contains("STR001"));
+    }
+
+    #[test]
+    fn search_roots_lists_charm_dir_then_repo_root() {
+        let dir = monorepo();
+        let charm_dir = dir.path().join("charms/test-charm");
+        let ctx = context::build_context(&charm_dir);
+        let repo_root = dir.path().canonicalize().unwrap();
+        assert_eq!(ctx.repo_root.as_deref(), Some(repo_root.as_path()));
+        assert_eq!(ctx.search_roots(), vec![ctx.charm_dir.as_path(), repo_root.as_path()]);
+    }
+
+    #[test]
+    fn search_roots_collapses_when_charm_is_the_repo_root() {
+        let dir = monorepo();
+        std::fs::create_dir(dir.path().join("src")).unwrap();
+        write(&dir.path().join("charmcraft.yaml"), "name: test-charm\n");
+        let ctx = context::build_context(dir.path());
+        assert_eq!(ctx.search_roots(), vec![ctx.charm_dir.as_path()]);
+    }
+
+    #[test]
+    fn search_roots_is_charm_dir_only_outside_a_repository() {
+        let dir = charm_with_yaml("name: test-charm\n");
+        let ctx = context::build_context(dir.path());
+        assert!(ctx.repo_root.is_none());
+        assert_eq!(ctx.search_roots(), vec![ctx.charm_dir.as_path()]);
+    }
 }

src/charmlint/linter.py

@@ -95,6 +95,19 @@ def _check_tests(charm_dir: pathlib.Path) -> tuple[bool, bool]:
     return has_unit, has_integration
 
 
+def _find_repo_root(charm_dir: pathlib.Path) -> pathlib.Path | None:
+    """Return the repository root enclosing ``charm_dir``, or None.
+
+    The nearest ancestor holding a ``.git`` entry wins.  ``.git`` is tested with
+    ``exists()`` rather than ``is_dir()`` because it is a plain file in linked
+    worktrees and submodules, which are repository roots just the same.
+    """
+    for candidate in (charm_dir, *charm_dir.parents):
+        if (candidate / ".git").exists():
+            return candidate
+    return None
+
+
 def build_context(charm_dir: pathlib.Path) -> models.CharmContext:
     """Load all charm data into a CharmContext for rule evaluation."""
     charm_dir = charm_dir.resolve()
@@ -135,6 +148,7 @@ def build_context(charm_dir: pathlib.Path) -> models.CharmContext:
 
     return models.CharmContext(
         charm_dir=charm_dir,
+        repo_root=_find_repo_root(charm_dir),
         metadata=metadata,
         actions=actions,
         config_options=config_options,

src/charmlint/models.py

@@ -66,6 +66,22 @@ class CharmContext:
     readme_content: str = ""
     has_tests_unit: bool = False
     has_tests_integration: bool = False
+    # Repository root enclosing ``charm_dir``, or None when the charm is not
+    # inside a repository.  Monorepos share repository-level assets (a single
+    # ``docs/`` tree, one licence) between charms, so rules that look for such
+    # assets must search here too.
+    repo_root: pathlib.Path | None = None
+
+    def search_roots(self) -> list[pathlib.Path]:
+        """Return the directories to search for repository-level assets.
+
+        The charm's own directory first, then the repository root when it is a
+        different directory.  Charm-local assets therefore still win, and a
+        charm outside a repository behaves exactly as before.
+        """
+        if self.repo_root is None or self.repo_root == self.charm_dir:
+            return [self.charm_dir]
+        return [self.charm_dir, self.repo_root]
 
 
 @dataclasses.dataclass

src/charmlint/rules/documentation.py

@@ -72,20 +72,27 @@ def _check_doc_topic(
     keyword: str,
     label: str,
 ) -> list[models.Diagnostic]:
-    """Check if a documentation topic is present in README or docs/."""
+    """Check if a documentation topic is present in README or a docs/ tree.
+
+    Monorepos commonly keep one ``docs/`` tree at the repository root, shared by
+    every charm under ``charms/<name>/``, so the repository root is searched as
+    well as the charm's own directory.
+    """
     # Check README.
     if keyword in context.readme_content.lower():
         return []
 
-    # Check docs/ directory.
-    docs_dir = context.charm_dir / "docs"
-    if docs_dir.is_dir():
+    # Check each docs/ directory — the charm's own, then the repository's.
+    for root in context.search_roots():
+        docs_dir = root / "docs"
+        if not docs_dir.is_dir():
+            continue
         for doc_file in docs_dir.rglob("*.md"):
             try:
                 content = doc_file.read_text(errors="replace").lower()
-                if keyword in content:
-                    return []
             except OSError:
                 continue
+            if keyword in content:
+                return []
 
     return [rule.diagnostic(f"No {label} documentation found")]

src/charmlint/rules/structure.py

@@ -15,12 +15,12 @@ class NoLicence(Rule):
     default_severity = models.Severity.INFO
 
     def check(self, context: models.CharmContext) -> list[models.Diagnostic]:
-        has_licence = (context.charm_dir / "LICENSE").exists() or (
-            context.charm_dir / "LICENCE"
-        ).exists()
-        if not has_licence:
-            return [self.diagnostic("No LICENSE/LICENCE file found")]
-        return []
+        # A monorepo normally carries one licence at the repository root rather
+        # than a copy in every charm directory.
+        for root in context.search_roots():
+            if (root / "LICENSE").exists() or (root / "LICENCE").exists():
+                return []
+        return [self.diagnostic("No LICENSE/LICENCE file found")]
 
 
 class NoIcon(Rule):