Skip to content
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.

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

Already on GitHub? Sign in to your account

feat: Remove extra error at interfaceExtendsObjectIntersectionErrors #1103

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

sunrabbit123
Copy link
Collaborator

Description:

interface I31<T> extends T { x: string }

I have a concern.

The following functions cause duplicate error handling due to duplicate function calls.

#[validator]
impl Analyzer<'_, '_> {
    fn validate(&mut self, d: &RTsInterfaceDecl) -> VResult<Type> {
        let ty = self.with_child(ScopeKind::Flow, Default::default(), |child: &mut Analyzer| -> VResult<_> {
            match &*d.id.sym {
                "any" | "void" | "never" | "unknown" | "string" | "number" | "bigint" | "boolean" | "null" | "undefined" | "symbol" => {
                    child.storage.report(ErrorKind::InvalidInterfaceName { span: d.id.span }.into());
                }
                _ => {}
            }

            let mut ty = Interface {
                span: d.span,
                name: d.id.clone().into(),
                type_params: try_opt!(d.type_params.validate_with(&mut *child).map(|v| v.map(Box::new))),
                extends: d.extends.validate_with(child)?.freezed(),
                body: d.body.validate_with(child)?,
                metadata: Default::default(),
                tracker: Default::default(),
            };
            child.prevent_expansion(&mut ty.body);
            ty.body.freeze();

            child.resolve_parent_interfaces(&d.extends, true);
            child.report_error_for_conflicting_parents(d.id.span, &ty.extends);
            child.report_error_for_wrong_interface_inheritance(d.id.span, &ty.body, &ty.extends);

            let ty = Type::Interface(ty).freezed();

            Ok(ty)
        })?;

        // TODO(kdy1): Recover
        self.register_type(d.id.clone().into(), ty.clone());

        Ok(ty)
    }
}

child.resolve_parent_interfaces and child.report_error_for_wrong_interface_inheritance call report_error_for_unresolved_type
this fn cause ErrorKind::TypeNotFound

This PR only clogs the hole.

If there seems to be a need for fundamental improvement, please open up the issue

BREAKING CHANGE:

Related issue (if exists):

Copy link

changeset-bot bot commented Nov 14, 2023

⚠️ No Changeset found

Latest commit: 99f79ac

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@sunrabbit123 sunrabbit123 changed the title feat: remove extra error feat: remove extra error at interfaceExtendsObjectIntersectionErrors Nov 14, 2023
Copy link

Thank you for the PR!
Commit: 99f79ac

Files to check

These are files which is affected by the current PR, but not reflected. If there's no file below this message, please ignore this message.

You can run ./scripts/auto-unignore.sh from crates/stc_ts_file_analyzer for typescript files, and ./scripts/check.sh from crates/stc_ts_type_checker for *.stats.rust-debug files.

@sunrabbit123 sunrabbit123 added this to the v0.0.1: Correctness milestone Nov 14, 2023
@sunrabbit123 sunrabbit123 self-assigned this Nov 14, 2023
@kdy1 kdy1 changed the title feat: remove extra error at interfaceExtendsObjectIntersectionErrors feat: Remove extra error at interfaceExtendsObjectIntersectionErrors Nov 14, 2023
@kdy1 kdy1 merged commit fa15470 into dudykr:main Nov 14, 2023
9 checks passed
@kdy1
Copy link
Member

kdy1 commented Nov 14, 2023

I think report_error_for_wrong_interface_inheritance should not call report_error_for_unresolved_type

@sunrabbit123
Copy link
Collaborator Author

sunrabbit123 commented Nov 14, 2023

umm... the flow of the function is as follows.

report_error_for_wrong_interface_inheritance -> self.type_of_ts_entity_name -> type_of_ts_entity_name_inner -> report_error_for_unresolved_type

Many places use self.type_of_ts_entity_name, making it difficult to modify it easily.
Do you know any other function that work similar to this?

@kdy1
Copy link
Member

kdy1 commented Nov 14, 2023

No, but we may use ctx to pass option to ignore resolving failure

@sunrabbit123
Copy link
Collaborator Author

I'll put it up as an issue for refactoring.

I can't think of a good way now.

Because a simple if statement returns Ref, which breaks the logic, is worse than not in consequence.

thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants