Impl warnings for empty inherits and legacy syntax

lsp/src/convert.rs

@@ -1,6 +1,7 @@
 use crate::{LineMap, StateSnapshot, Vfs, VfsPath};
 use lsp_types::{
-    self as lsp, DiagnosticSeverity, Location, Position, Range, TextDocumentPositionParams,
+    self as lsp, DiagnosticSeverity, DiagnosticTag, Location, Position, Range,
+    TextDocumentPositionParams,
 };
 use nil::{Diagnostic, FilePos, InFile, Severity};
 use text_size::TextRange;
@@ -32,6 +33,7 @@ pub(crate) fn to_diagnostic(line_map: &LineMap, diag: Diagnostic) -> Option<lsp:
     Some(lsp::Diagnostic {
         severity: match diag.severity() {
             Severity::Error => Some(DiagnosticSeverity::ERROR),
+            Severity::Warning => Some(DiagnosticSeverity::WARNING),
             Severity::IncompleteSyntax => return None,
         },
         range: to_range(line_map, diag.range),
@@ -40,7 +42,16 @@ pub(crate) fn to_diagnostic(line_map: &LineMap, diag: Diagnostic) -> Option<lsp:
         source: None,
         message: diag.message(),
         related_information: None,
-        tags: None,
+        tags: {
+            let mut tags = Vec::new();
+            if diag.is_deprecated() {
+                tags.push(DiagnosticTag::DEPRECATED);
+            }
+            if diag.is_unnecessary() {
+                tags.push(DiagnosticTag::UNNECESSARY);
+            }
+            Some(tags)
+        },
         data: None,
     })
 }

src/def/lower.rs

@@ -165,6 +165,7 @@ impl LowerCtx {
                 let (is_rec, ctor): (bool, fn(_) -> _) = if e.rec_token().is_some() {
                     (true, Expr::Attrset)
                 } else if e.let_token().is_some() {
+                    self.diagnostic(e.syntax().text_range(), DiagnosticKind::LetAttrset);
                     (true, Expr::LetAttrset)
                 } else {
                     (false, Expr::Attrset)
@@ -214,7 +215,10 @@ impl LowerCtx {
         Some(match kind {
             LiteralKind::Int => Literal::Int(text.parse::<i64>().ok()?),
             LiteralKind::Float => Literal::Float(text.parse::<f64>().unwrap().into()),
-            LiteralKind::Uri => Literal::String(text.into()),
+            LiteralKind::Uri => {
+                self.diagnostic(lit.syntax().text_range(), DiagnosticKind::UriLiteral);
+                Literal::String(text.into())
+            }
             LiteralKind::SearchPath => {
                 text = &text[1..text.len() - 1]; // Strip '<' and '>'.
                 let (search_name, relative_path) = text.split_once('/').unwrap_or((text, ""));
@@ -409,7 +413,10 @@ impl MergingSet {
             from_id
         });
 
+        let mut no_attrs = true;
         for attr in i.attrs() {
+            no_attrs = false;
+
             let ptr = AstPtr::new(attr.syntax());
             let key = match ctx.lower_key(self.is_rec, attr) {
                 // `inherit ${expr}` or `inherit (expr) ${expr}` is invalid.
@@ -446,6 +453,10 @@ impl MergingSet {
             };
             self.entries.insert(key, entry);
         }
+
+        if no_attrs {
+            ctx.diagnostic(i.syntax().text_range(), DiagnosticKind::EmptyInherit);
+        }
     }
 
     fn merge_path_value(
@@ -592,6 +603,12 @@ mod tests {
         let parse = parse_file(src);
         let (module, _source_map) = lower(InFile::new(FileId(0), parse));
         let mut got = String::new();
+        for diag in module.diagnostics() {
+            writeln!(got, "{:?}", diag).unwrap();
+        }
+        if !module.diagnostics.is_empty() {
+            writeln!(got).unwrap();
+        }
         for (i, e) in module.exprs.iter() {
             writeln!(got, "{}: {:?}", i.into_raw(), e).unwrap();
         }
@@ -601,7 +618,6 @@ mod tests {
         for (i, def) in module.name_defs.iter() {
             writeln!(got, "{}: {:?}", i.into_raw(), def).unwrap();
         }
-        assert!(module.diagnostics().is_empty());
         expect.assert_eq(&got);
     }
 
@@ -632,6 +648,8 @@ mod tests {
         check_lower(
             "a:b",
             expect![[r#"
+                Diagnostic { range: 0..3, kind: UriLiteral }
+
                 0: Literal(String("a:b"))
             "#]],
         );
@@ -821,6 +839,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn uri() {
+        check_lower(
+            "foo:bar",
+            expect![[r#"
+                Diagnostic { range: 0..7, kind: UriLiteral }
+
+                0: Literal(String("foo:bar"))
+            "#]],
+        );
+    }
+
     #[test]
     fn attrpath() {
         check_lower(
@@ -866,6 +896,9 @@ mod tests {
         check_lower(
             r#"{ inherit; inherit a "b" ${("c")}; inherit (d); inherit (e) f g; }"#,
             expect![[r#"
+                Diagnostic { range: 2..10, kind: EmptyInherit }
+                Diagnostic { range: 35..47, kind: EmptyInherit }
+
                 0: Reference("a")
                 1: Reference("b")
                 2: Reference("c")
@@ -966,6 +999,8 @@ mod tests {
         check_lower(
             "{ a.b = let { c.d = 1; }; }",
             expect![[r#"
+                Diagnostic { range: 8..24, kind: LetAttrset }
+
                 0: Literal(Int(1))
                 1: Attrset(Bindings { entries: [(Name("d"), Expr(Idx::<Expr>(0)))], inherit_froms: [] })
                 2: LetAttrset(Bindings { entries: [(NameDef(Idx::<NameDef>(0)), Expr(Idx::<Expr>(1)))], inherit_froms: [] })

src/diagnostic.rs

@@ -9,20 +9,31 @@ pub struct Diagnostic {
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum DiagnosticKind {
+    // Syntax.
     SyntaxError(SynErrorKind),
+
+    // Lowering.
     InvalidDynamic,
     DuplicatedKey,
+    EmptyInherit,
+    LetAttrset,
+    UriLiteral,
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]
 pub enum Severity {
     Error,
+    Warning,
     IncompleteSyntax,
 }
 
 impl Diagnostic {
     pub fn severity(&self) -> Severity {
         match self.kind {
+            DiagnosticKind::InvalidDynamic | DiagnosticKind::DuplicatedKey => Severity::Error,
+            DiagnosticKind::EmptyInherit
+            | DiagnosticKind::LetAttrset
+            | DiagnosticKind::UriLiteral => Severity::Warning,
             DiagnosticKind::SyntaxError(kind) => match kind {
                 SynErrorKind::MultipleRoots
                 | SynErrorKind::PathTrailingSlash
@@ -33,17 +44,35 @@ impl Diagnostic {
                 | SynErrorKind::MissingExpr
                 | SynErrorKind::MissingAttr => Severity::IncompleteSyntax,
             },
-            DiagnosticKind::InvalidDynamic | DiagnosticKind::DuplicatedKey => Severity::Error,
         }
     }
 
     pub fn message(&self) -> String {
         match self.kind {
             DiagnosticKind::SyntaxError(kind) => kind.to_string(),
+
             DiagnosticKind::InvalidDynamic => "Invalid location of dynamic attribute".into(),
             DiagnosticKind::DuplicatedKey => "Duplicated name definition".into(),
+            DiagnosticKind::EmptyInherit => "Nothing inherited".into(),
+            DiagnosticKind::LetAttrset => {
+                "`let { ... }` is deprecated. Use `let ... in ...` instead".into()
+            }
+            DiagnosticKind::UriLiteral => {
+                "URL literal is confusing and deprecated. Use strings instead".into()
+            }
         }
     }
+
+    pub fn is_unnecessary(&self) -> bool {
+        matches!(self.kind, DiagnosticKind::EmptyInherit)
+    }
+
+    pub fn is_deprecated(&self) -> bool {
+        matches!(
+            self.kind,
+            DiagnosticKind::LetAttrset | DiagnosticKind::UriLiteral
+        )
+    }
 }
 
 impl From<syntax::Error> for Diagnostic {