From ff6ea7a849cbed1ac4d5547913264d05921e081f Mon Sep 17 00:00:00 2001
From: Tim Fennis <fennis.tim@gmail.com>
Date: Tue, 13 Oct 2026 15:36:44 +0200
Subject: [PATCH] =?UTF-8?q?refactor(parser):=20give=20type=20annotation=20?=
 =?UTF-8?q?names=20node=20ids=20=F0=9F=8F=B7=EF=B8=8F?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Name resolution needs to record what each type name refers to. Each name
in `Map<P, Q>` resolves separately, and annotations on parameters have
no node of their own, so give every `TypeExpr::Name` its own `NodeId`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 ndc_analyser/src/analyser.rs         |  4 +-
 ndc_parser/src/parser.rs             |  2 +
 ndc_parser/src/type_expr.rs          |  4 ++
 ndc_parser/tests/type_annotations.rs | 88 ++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 ndc_parser/tests/type_annotations.rs

diff --git a/ndc_analyser/src/analyser.rs b/ndc_analyser/src/analyser.rs
index 0bd54eb7..395a40a5 100644
--- a/ndc_analyser/src/analyser.rs
+++ b/ndc_analyser/src/analyser.rs
@@ -112,7 +112,9 @@ impl Analyser {
             TypeExpr::Tuple { elements, .. } => {
                 StaticType::Tuple(elements.iter().map(|e| self.lower_type_expr(e)).collect())
             }
-            TypeExpr::Name { name, args, span } => {
+            TypeExpr::Name {
+                name, args, span, ..
+            } => {
                 let has_args = !args.is_empty();
                 let args = args.iter().map(|a| self.lower_type_expr(a)).collect();
                 match self.scope_tree.resolve_type(name).cloned() {
diff --git a/ndc_parser/src/parser.rs b/ndc_parser/src/parser.rs
index 1d8070ee..94887dc9 100644
--- a/ndc_parser/src/parser.rs
+++ b/ndc_parser/src/parser.rs
@@ -1458,6 +1458,7 @@ impl Parser {
             };
 
             return Ok(TypeExpr::Name {
+                id: NodeId::next(),
                 name,
                 args: Vec::new(),
                 span,
@@ -1547,6 +1548,7 @@ impl Parser {
         };
 
         Ok(TypeExpr::Name {
+            id: NodeId::next(),
             name: ident,
             args,
             span,
diff --git a/ndc_parser/src/type_expr.rs b/ndc_parser/src/type_expr.rs
index ae614518..d2f1172d 100644
--- a/ndc_parser/src/type_expr.rs
+++ b/ndc_parser/src/type_expr.rs
@@ -1,3 +1,4 @@
+use crate::expression::NodeId;
 use ndc_lexer::Span;
 use std::fmt;
 
@@ -8,6 +9,9 @@ use std::fmt;
 pub enum TypeExpr {
     /// A (possibly generic) named type: `Int`, `List<Int>`, `Map<String, Int>`.
     Name {
+        /// Identity of this name, independent of its source location. In
+        /// `Map<P, Q>`, `Map`, `P` and `Q` each have their own.
+        id: NodeId,
         name: String,
         args: Vec<Self>,
         span: Span,
diff --git a/ndc_parser/tests/type_annotations.rs b/ndc_parser/tests/type_annotations.rs
new file mode 100644
index 00000000..fabf4796
--- /dev/null
+++ b/ndc_parser/tests/type_annotations.rs
@@ -0,0 +1,88 @@
+#![allow(unused_crate_dependencies)]
+
+use std::collections::HashSet;
+
+use ndc_lexer::{Lexer, SourceId};
+use ndc_parser::{Expression, ExpressionLocation, NodeId, Parser, TypeExpr};
+
+fn parse_one(source: &str) -> ExpressionLocation {
+    let tokens = Lexer::new(source, SourceId::SYNTHETIC)
+        .collect::<Result<Vec<_>, _>>()
+        .unwrap();
+    let mut expressions = Parser::from_tokens(tokens).parse().unwrap();
+    assert_eq!(expressions.len(), 1);
+    expressions.pop().unwrap()
+}
+
+fn names(annotation: &TypeExpr, found: &mut Vec<(String, NodeId)>) {
+    match annotation {
+        TypeExpr::Name { id, name, args, .. } => {
+            found.push((name.clone(), *id));
+            for arg in args {
+                names(arg, found);
+            }
+        }
+        TypeExpr::Tuple { elements, .. } => {
+            for element in elements {
+                names(element, found);
+            }
+        }
+    }
+}
+
+fn assert_distinct(ids: impl IntoIterator<Item = NodeId>) {
+    let ids = ids.into_iter().collect::<Vec<_>>();
+    let unique = ids.iter().collect::<HashSet<_>>();
+    assert_eq!(unique.len(), ids.len(), "{ids:?}");
+}
+
+#[test]
+fn every_type_name_has_its_own_id() {
+    let expression = parse_one("let x: Map<P, (Q, List<P>)> = input");
+    let Expression::VariableDeclaration {
+        annotated_type: Some(annotation),
+        ..
+    } = expression.expression
+    else {
+        panic!("expected an annotated declaration");
+    };
+
+    let mut found = vec![];
+    names(&annotation, &mut found);
+    assert_eq!(
+        found
+            .iter()
+            .map(|(name, _)| name.as_str())
+            .collect::<Vec<_>>(),
+        ["Map", "P", "Q", "List", "P"]
+    );
+    // Both `P`s are separate uses, so they get separate ids.
+    assert_distinct(found.into_iter().map(|(_, id)| id));
+}
+
+#[test]
+fn cast_targets_before_a_comparison_have_ids() {
+    // `Int < 5` is not a type argument list, so the parser reads `Int` on its own.
+    let expression = parse_one("[x as Int < 5, y as Int < 6]");
+    let Expression::List { values } = expression.expression else {
+        panic!("expected a list");
+    };
+
+    let ids = values.iter().map(|value| {
+        let (Expression::Call { arguments, .. } | Expression::OperatorCall { arguments, .. }) =
+            &value.expression
+        else {
+            panic!("expected a comparison, got {value:?}");
+        };
+        let Expression::Cast {
+            annotation: TypeExpr::Name { id, name, .. },
+            ..
+        } = &arguments[0].expression
+        else {
+            panic!("expected a cast to a named type");
+        };
+        assert_eq!(name, "Int");
+        *id
+    });
+    assert_distinct(ids);
+}