Skip to content

Commit

Permalink
Implement excess property warnings (#139)
Browse files Browse the repository at this point in the history
  • Loading branch information
PatrickLaflamme authored May 17, 2024
1 parent 18564ca commit c1ecf40
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 10 deletions.
Binary file modified checker/definitions/internal.ts.d.bin
Binary file not shown.
58 changes: 54 additions & 4 deletions checker/specification/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,10 +150,10 @@ my_obj.a = "hello world"
#### Objects checks

```ts
const my_obj: { b: 3 } = { a: 2 }
const my_obj: { b: 3 } = { b: 4 }
```

- Type { a: 2 } is not assignable to type { b: 3 }
- Type { b: 4 } is not assignable to type { b: 3 }

#### Getters

Expand Down Expand Up @@ -211,6 +211,55 @@ const b = x.b;

- No property 'b' on { a: 2 }

#### Excess property at declaration

```ts
interface MyObject { property: string }

const a: MyObject = { property: "hello", another: 2 }
```

- Excess property 'another' was provided, but is not a property of MyObject

#### Excess property at argument

```ts
interface MyObject { property: string }

function process(param: MyObject) {}

process({ property: "hello", another: 2 })
```

- Excess property 'another' was provided, but is not a property of MyObject

#### Excess property checks through spread and condition

```ts
type MyObject = { foo: number; bar?: number };

const b: MyObject = {
foo: 1,
...{
bar: 2,
invalid: 3,
},
};

declare let condition: boolean;

const c: MyObject = {
foo: 1,
...(condition ? {
bar: 2,
non_existent: 3,
} : {}),
};
```

- Excess property 'invalid' was provided, but is not a property of MyObject
- Excess property 'non_existent' was provided, but is not a property of MyObject

### Constant evaluation

#### Arithmetic
Expand Down Expand Up @@ -2074,10 +2123,10 @@ function getA<T extends { a: string }>(p: T) {
return p.a
}

getA({ p: 2 })
getA({ a: 2 })
```

- Argument of type { p: 2 } is not assignable to parameter of type T
- Argument of type { a: 2 } is not assignable to parameter of type T

> I think reasons contains more information
Expand Down Expand Up @@ -2445,6 +2494,7 @@ const x: Record2<"test", boolean> = { no: false },
z: Record2<"test", boolean> = { test: false };
```

- Excess property 'no' was provided, but is not a property of { [\"test\"]: boolean }
- Type { no: false } is not assignable to type { [\"test\"]: boolean }
- Type { test: 6 } is not assignable to type { [\"test\"]: boolean }

Expand Down
10 changes: 10 additions & 0 deletions checker/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,11 @@ mod defined_errors_and_warnings {
expression_span: SpanWithSource,
expression_value: bool,
},
ExcessProperty {
position: SpanWithSource,
expected_type: TypeStringRepresentation,
excess_property_name: String,
},
IgnoringAsExpression(SpanWithSource),
Unimplemented {
thing: &'static str,
Expand Down Expand Up @@ -911,6 +916,11 @@ mod defined_errors_and_warnings {
kind,
}
}
TypeCheckWarning::ExcessProperty {
position,
expected_type,
excess_property_name,
} => Diagnostic::Position { reason: format!("Excess property '{excess_property_name}' was provided, but is not a property of {expected_type}"), position, kind },
TypeCheckWarning::IgnoringAsExpression(position) => Diagnostic::Position {
reason: "'as' expressions are ignore by the checker".to_owned(),
position,
Expand Down
39 changes: 35 additions & 4 deletions checker/src/synthesis/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ use parser::{
TemplateLiteral,
},
functions::MethodHeader,
ASTNode, Expression,
ASTNode, Expression, ToStringOptions,
};

use crate::{
context::{
information::{get_properties_on_type, get_property_unbound},
Logical,
},
diagnostics::{TypeCheckError, TypeStringRepresentation},
diagnostics::{TypeCheckError, TypeCheckWarning, TypeStringRepresentation},
features::{
self, await_expression,
functions::{
Expand Down Expand Up @@ -1048,14 +1048,45 @@ pub(super) fn synthesise_object_literal<T: crate::ReadFromFS>(
Some(member_position),
);
}
ObjectLiteralMember::Property { key, value, .. } => {
ObjectLiteralMember::Property { key: k, value, position, .. } => {
let key = parser_property_key_to_checker_property_key(
key.get_ast_ref(),
k.get_ast_ref(),
environment,
checking_data,
true,
);

let position_with_source = position.with_source(environment.get_source());

let maybe_property_expecting = environment.get_property(
expected,
Publicity::Public,
&key,
&mut checking_data.types,
None,
position_with_source,
&checking_data.options,
false,
);

if expected != TypeId::ANY_TYPE
&& expected != TypeId::OBJECT_TYPE
&& maybe_property_expecting.is_none()
{
checking_data.diagnostics_container.add_warning(
TypeCheckWarning::ExcessProperty {
position: position_with_source,
expected_type: TypeStringRepresentation::from_type_id(
expected,
environment,
&checking_data.types,
checking_data.options.debug_types,
),
excess_property_name: k.to_string(&ToStringOptions::default()),
},
);
}

// TODO needs improvement
let property_expecting = get_property_unbound(
expected,
Expand Down
34 changes: 32 additions & 2 deletions checker/src/types/properties.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use source_map::SpanWithSource;

use super::{calling::CalledWithNew, Constructor, Type, TypeStore};

#[derive(PartialEq)]
pub enum PropertyKind {
Direct,
Getter,
Expand Down Expand Up @@ -376,10 +377,39 @@ fn get_from_an_object<E: CallCheckingBehavior>(
}
PropertyValue::Setter(_) => todo!(),
PropertyValue::Deleted => None,
PropertyValue::Dependent { .. } => todo!(),
PropertyValue::Dependent { on, truthy: _, otherwise: _ } => {
// TODO: why does this work?
Some((PropertyKind::Direct, on))
}
}
}
Logical::Or { .. } => todo!(),
Logical::Or { left, right, based_on } => left
.map(|l| {
resolve_property_on_logical(
l,
based_on,
None,
environment,
types,
behavior,
bind_this,
)
})
.or_else(|_| {
right.map(|r| {
resolve_property_on_logical(
r,
based_on,
None,
environment,
types,
behavior,
bind_this,
)
})
})
.ok()
.flatten(),
Logical::Implies { on: log_on, antecedent } => resolve_property_on_logical(
*log_on,
on,
Expand Down

0 comments on commit c1ecf40

Please sign in to comment.