Skip to content

fix(analyser): preserve operand types in augmented assignments 馃攷 #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
timfennis wants to merge 3 commits into from
Closed

fix(analyser): preserve operand types in augmented assignments 馃攷 #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f78e535
fix(analyser): explain augmented assignment type mismatches 馃攷
timfennis Sep 8, 2026
83eed80
fix(analyser): retain operand types during vector fallback 馃攷
timfennis Sep 9, 2026
c90a641
test(cli): remove diagnostic unit tests 馃Ч
timfennis Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 69 additions & 84 deletions ndc_analyser/src/analyser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ impl Analyser {
}
Expression::OpAssignment {
l_value,
l_value_span,
r_value,
operation,
plan,
Expand Down Expand Up @@ -402,10 +403,12 @@ impl Analyser {
// change the concrete left type. Reject it here rather
// than falling through to an ordinary operation whose
// erased return type could widen the same target.
self.emit(AnalysisError::mismatched_types(
self.emit(AnalysisError::augmented_operand_mismatch(
&right_type,
&left_type,
*span,
r_value.span,
*l_value_span,
operation,
));
*plan = AugmentedAssignmentPlan::Unresolved;
None
Expand Down Expand Up @@ -1270,6 +1273,8 @@ pub struct AnalysisError {
text: String,
span: Span,
help_text: Option<String>,
primary_label: Option<String>,
related_labels: Vec<(Span, String)>,
}

impl AnalysisError {
Expand All @@ -1283,11 +1288,41 @@ impl AnalysisError {
self.help_text.as_deref()
}

/// Label for the primary span, when the error can identify its role.
pub fn primary_label(&self) -> Option<&str> {
self.primary_label.as_deref()
}

/// Other source locations that explain the primary error.
pub fn related_labels(&self) -> &[(Span, String)] {
&self.related_labels
}

fn augmented_operand_mismatch(
found: &StaticType,
expected: &StaticType,
right_span: Span,
left_span: Span,
operation: &str,
) -> Self {
let mut error = Self::mismatched_types(found, expected, right_span);
error.primary_label = Some(format!("right operand inferred as {found}"));
error.related_labels.push((
left_span,
format!(
"left operand has type {expected}; `{operation}=` requires a compatible right operand"
),
));
error
}

fn invalid_type_annotation(err: &StaticTypeConstructionError, span: Span) -> Self {
Self {
text: err.to_string(),
span,
help_text: Some(err.help_text().to_string()),
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1296,6 +1331,8 @@ impl AnalysisError {
text: format!("type `{name}` does not take generic arguments"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1304,6 +1341,8 @@ impl AnalysisError {
text: format!("Struct '{name}' is not allowed to shadow the built-in type '{name}'"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1312,6 +1351,8 @@ impl AnalysisError {
text: format!("Illegal redefinition of struct '{name}'"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1320,6 +1361,8 @@ impl AnalysisError {
text: format!("Illegal redefinition of field '{field}' in struct '{struct_name}'"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1330,6 +1373,8 @@ impl AnalysisError {
),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1338,6 +1383,8 @@ impl AnalysisError {
text: format!("mismatched types: found {found} but expected {expected}"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1346,6 +1393,8 @@ impl AnalysisError {
text: format!("invalid cast: {found} can never be {target}"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1360,6 +1409,8 @@ impl AnalysisError {
),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1368,27 +1419,35 @@ impl AnalysisError {
text: format!("Illegal redefinition of parameter {param}"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}
fn unable_to_index_into(typ: &StaticType, span: Span) -> Self {
Self {
text: format!("Unable to index into {typ}"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}
fn unable_to_unpack_type(typ: &StaticType, span: Span) -> Self {
Self {
text: format!("Invalid unpacking of {typ}"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}
fn lvalue_required_to_be_single_identifier(span: Span) -> Self {
Self {
text: "This lvalue is required to be a single identifier".to_string(),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1400,6 +1459,8 @@ impl AnalysisError {
),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1423,6 +1484,8 @@ impl AnalysisError {
text: format!("Unable to invoke {typ} as a function."),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}

Expand All @@ -1431,6 +1494,8 @@ impl AnalysisError {
text: format!("Identifier {ident} has not previously been declared"),
span,
help_text: None,
primary_label: None,
related_labels: Vec::new(),
}
}
}
Expand Down Expand Up @@ -1515,88 +1580,8 @@ mod tests {
}

#[test]
fn inferred_index_augmented_assignment_widens_element_type() {
let add = StaticType::Function {
parameters: Some(vec![StaticType::Int, StaticType::Float]),
return_type: Box::new(StaticType::Number),
};
assert_eq!(
analyse_last_type_with_globals(
"let values = [1]; values[0] += 0.5; values",
vec![("+".to_string(), add)],
),
StaticType::List(Box::new(StaticType::Any)),
);
}

#[test]
fn inferred_identifier_assignments_widen_subsequent_reads() {
assert_eq!(analyse_last_type("let x = 3; x = 0.5; x"), StaticType::Any,);

let add = StaticType::Function {
parameters: Some(vec![StaticType::Int, StaticType::Float]),
return_type: Box::new(StaticType::Float),
};
assert_eq!(
analyse_last_type_with_globals("let x = 3; x += 0.5; x", vec![("+".to_string(), add)],),
StaticType::Any,
);
}

#[test]
fn annotated_identifier_augmented_assignment_rejects_widening() {
let add = StaticType::Function {
parameters: Some(vec![StaticType::Int, StaticType::Float]),
return_type: Box::new(StaticType::Float),
};
assert_analysis_error(
"let x: Int = 3; x += 0.5;",
vec![("+".to_string(), add)],
"mismatched types: found Float but expected Int",
);
}

#[test]
fn compatible_specialized_assignment_preserves_left_type() {
let list_any = StaticType::List(Box::new(StaticType::Any));
let append = StaticType::Function {
parameters: Some(vec![list_any.clone(), list_any.clone()]),
return_type: Box::new(list_any),
};

assert_eq!(
analyse_last_type_with_globals(
"let values = [1]; values ++= [2]; values",
vec![("++=".to_string(), append)],
),
StaticType::List(Box::new(StaticType::Int)),
);
}

#[test]
fn incompatible_specialized_assignment_is_rejected() {
let list_any = StaticType::List(Box::new(StaticType::Any));
let concat = StaticType::Function {
parameters: Some(vec![list_any.clone(), list_any.clone()]),
return_type: Box::new(list_any),
};

assert_analysis_error(
"let values = [1]; values ++= [\"two\"];",
vec![
("++=".to_string(), concat.clone()),
("++".to_string(), concat.clone()),
],
"mismatched types: found List<String> but expected List<Int>",
);
assert_analysis_error(
"let values: List<Int> = [1]; values ++= [\"two\"];",
vec![
("++=".to_string(), concat.clone()),
("++".to_string(), concat),
],
"mismatched types: found List<String> but expected List<Int>",
);
fn inferred_identifier_assignment_widens_subsequent_reads() {
assert_eq!(analyse_last_type("let x = 3; x = 0.5; x"), StaticType::Any);
}

#[test]
Expand Down
24 changes: 22 additions & 2 deletions ndc_analyser/src/scope.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use itertools::Itertools;
use ndc_core::StaticType;
use ndc_core::r#struct::StructInfo;
use ndc_parser::{Binding, Candidate, CaptureSource, ResolvedVar};
Expand Down Expand Up @@ -809,8 +810,27 @@ impl ScopeTree {
if !sig.iter().any(|t| matches!(t, StaticType::Any)) {
return None;
}
let permissive: Vec<StaticType> = vec![StaticType::Any; sig.len()];
let vars = self.candidates_for_sig(ident, &permissive);
// An Any argument may be a tuple at runtime, but known scalar
// arguments still constrain every element-wise call. Erasing those
// types would admit map mutation overloads for e.g. Bool |= Any.
// Sequence<T> may also hide a tuple: it can either be broadcast as
// one value or supply T elements. Keep both interpretations without
// erasing the constraints on the other arguments.
let signatures = sig
.iter()
.map(|arg| match arg {
StaticType::Sequence(element) => vec![arg.clone(), *element.clone()],
_ => vec![arg.clone()],
})
.multi_cartesian_product();
let mut vars = Vec::new();
for signature in signatures {
for var in self.candidates_for_sig(ident, &signature) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain overlapping container candidates in vector fallback

When a known operand is Sequence<T>, candidates_for_sig only retains overload parameters subtype-comparable with either Sequence<T> or T; it misses overlapping concrete containers such as List<Any>, even though a Sequence<Int> value may be a List<Int>. Consequently, fn concat(xs: Sequence<Int>, rhs) => xs ++ rhs; concat([1], ([2], [3])) is now rejected during analysis, although at runtime the list can be broadcast across the unknown tuple and the List ++ List overload accepts each element. The previous permissive fallback included this valid candidate, so the new filtering should account for overlapping sequence-family types while still excluding genuinely incompatible scalar overloads.

Useful? React with 馃憤聽/ 馃憥.

if !vars.contains(&var) {
vars.push(var);
}
}
}
if vars.is_empty() {
None
} else {
Expand Down
12 changes: 8 additions & 4 deletions ndc_bin/src/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,17 @@ fn into_diagnostics(err: InterpreterError) -> Vec<Diagnostic<SourceId>> {
.iter()
.map(|cause| {
let span = cause.span();
let mut labels = vec![
Label::primary(span.source_id(), span.range())
.with_message(cause.primary_label().unwrap_or("related to this")),
];
labels.extend(cause.related_labels().iter().map(|(span, message)| {
Label::secondary(span.source_id(), span.range()).with_message(message)
}));
let mut d = Diagnostic::error()
.with_code("resolver")
.with_message(cause.to_string())
.with_labels(vec![
Label::primary(span.source_id(), span.range())
.with_message("related to this"),
]);
.with_labels(labels);
if let Some(help) = cause.help_text() {
d = d.with_notes(vec![help.to_owned()]);
}
Expand Down
2 changes: 1 addition & 1 deletion ndc_lsp/src/backend.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl Backend {
.ok()
.map(|(expressions, analysis_result)| {
for err in &analysis_result.errors {
diagnostics.push(diagnostics::analysis_error_to_diagnostic(text, err));
diagnostics.push(diagnostics::analysis_error_to_diagnostic(text, uri, err));
}
(expressions, analysis_result)
})
Expand Down
Loading
Loading