Inherit context from source sites

crates/oak_db/src/diagnostic.rs

@@ -1,4 +1,5 @@
 use biome_rowan::TextRange;
+use biome_rowan::TextSize;
 use oak_semantic::semantic_index::AmbiguityReason;
 use oak_semantic::semantic_index::SemanticDiagnostic;
 
@@ -57,6 +58,8 @@ pub enum DiagnosticKind {
     AmbiguousEffect,
     AmbiguousAttachOrder,
     UninstalledPackage,
+    SourceCycle,
+    InheritedShadow,
 }
 
 impl DiagnosticKind {
@@ -66,6 +69,8 @@ impl DiagnosticKind {
             DiagnosticKind::AmbiguousEffect => "ambiguous-effect",
             DiagnosticKind::AmbiguousAttachOrder => "ambiguous-attach-order",
             DiagnosticKind::UninstalledPackage => "uninstalled-package",
+            DiagnosticKind::SourceCycle => "source-cycle",
+            DiagnosticKind::InheritedShadow => "inherited-shadow",
         }
     }
 
@@ -74,6 +79,8 @@ impl DiagnosticKind {
             DiagnosticKind::AmbiguousEffect => Severity::Info,
             DiagnosticKind::AmbiguousAttachOrder => Severity::Info,
             DiagnosticKind::UninstalledPackage => Severity::Warning,
+            DiagnosticKind::SourceCycle => Severity::Warning,
+            DiagnosticKind::InheritedShadow => Severity::Info,
         }
     }
 
@@ -82,6 +89,8 @@ impl DiagnosticKind {
             DiagnosticKind::AmbiguousEffect => true,
             DiagnosticKind::AmbiguousAttachOrder => true,
             DiagnosticKind::UninstalledPackage => true,
+            DiagnosticKind::SourceCycle => true,
+            DiagnosticKind::InheritedShadow => true,
         }
     }
 }
@@ -108,6 +117,7 @@ pub(crate) fn lower_semantic_diagnostic(diagnostic: &SemanticDiagnostic) -> Diag
         SemanticDiagnostic::UninstalledPackage { package, range } => {
             lower_uninstalled_package(package, *range)
         },
+        SemanticDiagnostic::SourceCycle => lower_source_cycle(),
     }
 }
 
@@ -188,3 +198,16 @@ fn lower_uninstalled_package(package: &str, range: TextRange) -> Diagnostic {
         Vec::new(),
     )
 }
+
+/// Anchored at the start of the file because the record carries no range.
+/// Every file in the cycle gets its own diagnostic.
+fn lower_source_cycle() -> Diagnostic {
+    Diagnostic::new(
+        DiagnosticKind::SourceCycle,
+        "This file takes part in a cycle of mutual `source()` calls. \
+         Language analysis will be incomplete until the cycle is resolved."
+            .to_string(),
+        TextRange::empty(TextSize::from(0)),
+        Vec::new(),
+    )
+}

crates/oak_db/src/file.rs

@@ -1,11 +1,13 @@
 use std::fs;
 
 use aether_path::FilePath;
+use oak_semantic::semantic_index::SemanticDiagnostic;
 use oak_semantic::semantic_index::SemanticIndex;
 
 use crate::db::root_by_file;
 use crate::diagnostic::lower_semantic_diagnostic;
 use crate::diagnostic::Diagnostic;
+use crate::file_diagnostics::inherited_shadow_diagnostics;
 use crate::file_revision::report_untracked_if_zero;
 use crate::imports::SalsaImportsResolver;
 use crate::parse::OakParse;
@@ -153,24 +155,22 @@ impl File {
     /// dependency graph through both `semantic_index` and `exports`:
     /// `semantic_index(A) -> SalsaImportsResolver -> exports(B) ->
     /// semantic_index(B) -> SalsaImportsResolver -> exports(A) ->
-    /// semantic_index(A)`. Salsa picks one query to break the cycle and
-    /// panics with "set cycle_fn/cycle_initial" unless that query has a
-    /// handler. Both `semantic_index` and the narrow queries (`exports`,
-    /// `imports`, `resolve`) carry their own `cycle_result`.
+    /// semantic_index(A)`.
     ///
     /// The two handlers behave differently:
     ///
-    /// - `semantic_index` (this query, custom rebuild): the cycling
-    ///   side is rebuilt with `NoopImportsResolver`. Cross-file
-    ///   injection drops, but local analysis (scopes, use-def maps,
-    ///   function bodies) is preserved.
+    /// - `semantic_index` (this query, custom rebuild): the file is rebuilt
+    ///   with `NoopImportsResolver`. Cross-file injection drops, but local
+    ///   analysis (scopes, use-def maps, function bodies) is preserved.
     ///
-    /// - `exports` / `imports` / `resolve` (FallbackImmediate, empty):
-    ///   the cycling side gets an empty fallback for that query.
+    /// - `exports` (FallbackImmediate, empty): the file contributes no names
+    ///   for the revision.
     ///
-    /// Which handler fires depends on which query salsa first re-enters.
-    /// R doesn't allow cyclic `source()`, so the asymmetric recovery is
-    /// acceptable. TODO(diagnostics): Lint `source()` cycles.
+    /// `FallbackImmediate` causes *every* participant in the cycle to fall
+    /// back, not just the one salsa re-entered. This means both ends of a
+    /// mutual `source()` pair rebuild under Noop and none of the source effects
+    /// resolve. A [`SemanticDiagnostic::SourceCycle`] is raised to document
+    /// this state.
     ///
     /// `no_eq` skips salsa's `values_equal` check after recomputation.
     /// Backdating at this level never triggered in practice anyway: `AstPtr`
@@ -252,7 +252,7 @@ impl File {
         names
     }
 
-    /// Diagnostics derived from this file's semantic index.
+    /// Diagnostics for this file.
     ///
     /// Not keyed on user configuration, so the memo isn't duplicated per
     /// setting combination. Consumers filter on severity and
@@ -262,11 +262,15 @@ impl File {
     /// `attached_packages()` and friends this query can't backdate.
     #[salsa::tracked(returns(ref))]
     pub fn diagnostics(self, db: &dyn Db) -> Vec<Diagnostic> {
-        self.semantic_index(db)
+        let mut diagnostics: Vec<Diagnostic> = self
+            .semantic_index(db)
             .diagnostics()
             .iter()
             .map(lower_semantic_diagnostic)
-            .collect()
+            .collect();
+
+        diagnostics.extend(inherited_shadow_diagnostics(db, self));
+        diagnostics
     }
 
     /// The root containing this file, if any.
@@ -349,6 +353,7 @@ fn semantic_index_cycle_result(db: &dyn Db, _id: salsa::Id, file: File) -> Seman
     );
     let parsed = file.parse(db);
     oak_semantic::build_index(&parsed.tree(), oak_semantic::NoopImportsResolver)
+        .with_diagnostic(SemanticDiagnostic::SourceCycle)
 }
 
 /// Test-only recorder for the deepest `build_semantic_index` nesting.

crates/oak_db/src/file_diagnostics.rs

@@ -0,0 +1,137 @@
+use rustc_hash::FxHashSet;
+
+use crate::diagnostic::Diagnostic;
+use crate::diagnostic::DiagnosticKind;
+use crate::file_imports::CollationView;
+use crate::file_imports::ImportLayer;
+use crate::file_resolve::resolve_import_layer;
+use crate::Db;
+use crate::File;
+use crate::Name;
+
+/// Diagnostics for effect ambiguities produced by source inheritance.
+///
+/// ```r
+/// # main.R                     # helpers.R
+/// source <- identity           source("more.R")
+/// base::source("helpers.R")
+/// ```
+///
+/// `helpers.R` is analysed on its own, so its `source("more.R")` reads as base
+/// `source` and we follow it. But `main.R` binds `source` to `identity` before
+/// sourcing `helpers.R`, so at runtime that call might do nothing at all.
+/// We report this ambiguity with a diagnostic.
+///
+/// A call is ambiguous when its callee resolves through [`File::imports`] (which
+/// includes context inherited from the sourcing files) to a layer absent from
+/// [`File::standalone_imports`] (the narrower view without inheritance).
+pub(crate) fn inherited_shadow_diagnostics(db: &dyn Db, file: File) -> Vec<Diagnostic> {
+    if file.inherited_layers(db, CollationView::Lazy).is_empty() {
+        return Vec::new();
+    }
+
+    let inherited = file.imports(db);
+    let standalone = file.standalone_imports(db);
+
+    let mut reported = FxHashSet::default();
+    let mut diagnostics = Vec::new();
+
+    for call in file.semantic_index(db).semantic_calls() {
+        let Some(callee) = call.callee() else {
+            continue;
+        };
+        // One `source()` call forwards one `Attach` per package attached in the
+        // target, all sharing its range. Report the site once.
+        if !reported.insert(call.range()) {
+            continue;
+        }
+
+        let name = Name::new(db, callee);
+
+        // A binding in this file wins on both sides, so there's nothing to
+        // disagree about. This is the same first step `File::resolve` takes.
+        if !file.resolve_export(db, name).is_empty() {
+            continue;
+        }
+
+        let Some(resolved_layer) = resolve_layer(db, inherited, name) else {
+            continue;
+        };
+
+        if standalone.contains(&resolved_layer) {
+            continue;
+        }
+
+        // A layer the standalone view lacks can only have come from an
+        // inherited band, so the site is always there to find.
+        let Some(sourcing) = sourcing_file(db, file, &resolved_layer) else {
+            continue;
+        };
+
+        // When nothing on the standalone path binds the callee, the scan fell
+        // through to base's builtins, which resolve by name whether or not base
+        // was scanned into a root. See `SalsaImportsResolver`.
+        let alone = match resolve_layer(db, &standalone, name) {
+            Some(layer) => describe_source(db, &layer),
+            None => "package `base`".to_string(),
+        };
+
+        diagnostics.push(Diagnostic::new(
+            DiagnosticKind::InheritedShadow,
+            format!(
+                "This `{callee}` call has an ambiguous effect. It resolves through {alone} when \
+                 the file is sourced on its own, and to {inherited} when sourced by `{sourcing}`.",
+                inherited = describe_source(db, &resolved_layer),
+            ),
+            call.range(),
+            Vec::new(),
+        ));
+    }
+
+    diagnostics
+}
+
+/// The first layer that binds `name`, i.e. the one a lookup would settle on.
+fn resolve_layer<'db>(
+    db: &'db dyn Db,
+    layers: &[ImportLayer],
+    name: Name<'db>,
+) -> Option<ImportLayer> {
+    layers
+        .iter()
+        .find(|layer| !resolve_import_layer(db, layer, name).is_empty())
+        .cloned()
+}
+
+/// The name of the file whose inherited band contributed `layer`.
+///
+/// Names the direct sourcing file even for a layer that reaches `file` from
+/// further up the chain, since [`build_inherited_layers`] folds each site's
+/// grandparents into that site's own bands.
+///
+/// [`build_inherited_layers`]: crate::file_imports
+fn sourcing_file(db: &dyn Db, file: File, layer: &ImportLayer) -> Option<String> {
+    file.inherited_layers(db, CollationView::Lazy)
+        .iter()
+        .find(|site| site.layers.above.contains(layer) || site.layers.below.contains(layer))
+        .map(|site| {
+            site.file
+                .path(db)
+                .file_name()
+                .unwrap_or_default()
+                .to_string()
+        })
+}
+
+fn describe_source(db: &dyn Db, layer: &ImportLayer) -> String {
+    match layer {
+        ImportLayer::File(file) | ImportLayer::SourcingFile { file, .. } => {
+            format!(
+                "a binding in `{}`",
+                file.path(db).file_name().unwrap_or_default()
+            )
+        },
+        ImportLayer::Package(package) => format!("package `{}`", package.name(db)),
+        ImportLayer::From(importer) => format!("an import of `{}`", importer.name(db)),
+    }
+}