From b9bf54494f1783daaa3a7d20e364e76aa0e32a85 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Tue, 10 Sep 2024 16:22:08 +0000 Subject: [PATCH] fix(isolated-declarations): false positive for setter method in `interface` (#5681) close: #5668 --- .../src/declaration.rs | 43 ++++++++++----- crates/oxc_isolated_declarations/src/lib.rs | 1 + .../src/signatures.rs | 52 +++++++++++++++++++ .../tests/fixtures/signatures.ts | 13 +++++ .../tests/snapshots/signatures.snap | 25 +++++++++ 5 files changed, 122 insertions(+), 12 deletions(-) create mode 100644 crates/oxc_isolated_declarations/src/signatures.rs create mode 100644 crates/oxc_isolated_declarations/tests/fixtures/signatures.ts create mode 100644 crates/oxc_isolated_declarations/tests/snapshots/signatures.snap diff --git a/crates/oxc_isolated_declarations/src/declaration.rs b/crates/oxc_isolated_declarations/src/declaration.rs index c500f44879504..e0c29c9d141ff 100644 --- a/crates/oxc_isolated_declarations/src/declaration.rs +++ b/crates/oxc_isolated_declarations/src/declaration.rs @@ -1,12 +1,16 @@ use std::cell::Cell; use oxc_allocator::Box; +use oxc_allocator::CloneIn; +use oxc_allocator::Vec; #[allow(clippy::wildcard_imports)] use oxc_ast::ast::*; -use oxc_ast::{syntax_directed_operations::BoundNames, Visit}; +use oxc_ast::visit::walk_mut::walk_ts_signatures; +use oxc_ast::{syntax_directed_operations::BoundNames, Visit, VisitMut}; use oxc_span::{GetSpan, SPAN}; use oxc_syntax::scope::ScopeFlags; +use crate::diagnostics::accessor_must_have_explicit_return_type; use crate::{ diagnostics::{ binding_element_export, inferred_type_of_expression, signature_computed_property_name, @@ -217,19 +221,19 @@ impl<'a> IsolatedDeclarations<'a> { } } Declaration::TSTypeAliasDeclaration(alias_decl) => { - self.visit_ts_type_alias_declaration(alias_decl); if !check_binding || self.scope.has_reference(&alias_decl.id.name) { - // SAFETY: `ast.copy` is unsound! We need to fix. - Some(unsafe { self.ast.copy(decl) }) + let mut decl = decl.clone_in(self.ast.allocator); + self.visit_declaration(&mut decl); + Some(decl) } else { None } } Declaration::TSInterfaceDeclaration(interface_decl) => { - self.visit_ts_interface_declaration(interface_decl); if !check_binding || self.scope.has_reference(&interface_decl.id.name) { - // SAFETY: `ast.copy` is unsound! We need to fix. - Some(unsafe { self.ast.copy(decl) }) + let mut decl = decl.clone_in(self.ast.allocator); + self.visit_declaration(&mut decl); + Some(decl) } else { None } @@ -286,15 +290,30 @@ impl<'a> IsolatedDeclarations<'a> { } } -impl<'a> Visit<'a> for IsolatedDeclarations<'a> { - fn visit_ts_method_signature(&mut self, signature: &TSMethodSignature<'a>) { +impl<'a> VisitMut<'a> for IsolatedDeclarations<'a> { + fn visit_ts_signatures(&mut self, signatures: &mut Vec<'a, TSSignature<'a>>) { + self.transform_ts_signatures(signatures); + walk_ts_signatures(self, signatures); + } + + fn visit_ts_method_signature(&mut self, signature: &mut TSMethodSignature<'a>) { + self.report_signature_property_key(&signature.key, signature.computed); if signature.return_type.is_none() { - self.error(inferred_type_of_expression(signature.span)); + match signature.kind { + TSMethodSignatureKind::Method => { + self.error(inferred_type_of_expression(signature.span)); + } + TSMethodSignatureKind::Get => { + self.error(accessor_must_have_explicit_return_type(signature.key.span())); + } + TSMethodSignatureKind::Set => { + // setter method don't need return type + } + } } - self.report_signature_property_key(&signature.key, signature.computed); } - fn visit_ts_property_signature(&mut self, signature: &TSPropertySignature<'a>) { + fn visit_ts_property_signature(&mut self, signature: &mut TSPropertySignature<'a>) { self.report_signature_property_key(&signature.key, signature.computed); } } diff --git a/crates/oxc_isolated_declarations/src/lib.rs b/crates/oxc_isolated_declarations/src/lib.rs index 079cdb6f5b811..f886c4d477fb3 100644 --- a/crates/oxc_isolated_declarations/src/lib.rs +++ b/crates/oxc_isolated_declarations/src/lib.rs @@ -16,6 +16,7 @@ mod literal; mod module; mod return_type; mod scope; +mod signatures; mod types; use std::{cell::RefCell, collections::VecDeque, mem}; diff --git a/crates/oxc_isolated_declarations/src/signatures.rs b/crates/oxc_isolated_declarations/src/signatures.rs new file mode 100644 index 0000000000000..bd7d6a61e057b --- /dev/null +++ b/crates/oxc_isolated_declarations/src/signatures.rs @@ -0,0 +1,52 @@ +use oxc_allocator::{CloneIn, Vec}; +use oxc_ast::ast::{TSMethodSignatureKind, TSSignature}; +use rustc_hash::FxHashMap; + +use crate::IsolatedDeclarations; + +impl<'a> IsolatedDeclarations<'a> { + /// Transform setter signature or getter return type to match the other + /// + /// Infer get accessor return type from set accessor's param type + /// Infer set accessor parameter type from get accessor return type + pub fn transform_ts_signatures(&mut self, signatures: &mut Vec<'a, TSSignature<'a>>) { + // + let mut method_annotations: FxHashMap<_, (bool, _, _)> = FxHashMap::default(); + + signatures.iter_mut().for_each(|signature| { + if let TSSignature::TSMethodSignature(method) = signature { + let Some(name) = method.key.static_name() else { + return; + }; + match method.kind { + TSMethodSignatureKind::Method => {} + TSMethodSignatureKind::Set => { + let Some(first_param) = method.params.items.first_mut() else { + return; + }; + + let entry = method_annotations.entry(name).or_default(); + entry.0 |= first_param.pattern.type_annotation.is_some(); + entry.1 = Some(&mut first_param.pattern.type_annotation); + } + TSMethodSignatureKind::Get => { + let entry = method_annotations.entry(name).or_default(); + entry.0 |= method.return_type.is_some(); + entry.2 = Some(&mut method.return_type); + } + }; + } + }); + + for (requires_inference, param, return_type) in method_annotations.into_values() { + if !requires_inference { + return; + } + if let (Some(Some(annotation)), Some(option)) | (Some(option), Some(Some(annotation))) = + (param, return_type) + { + option.replace(annotation.clone_in(self.ast.allocator)); + } + } + } +} diff --git a/crates/oxc_isolated_declarations/tests/fixtures/signatures.ts b/crates/oxc_isolated_declarations/tests/fixtures/signatures.ts new file mode 100644 index 0000000000000..57efd21518f3d --- /dev/null +++ b/crates/oxc_isolated_declarations/tests/fixtures/signatures.ts @@ -0,0 +1,13 @@ +export interface X { + set value(_: string); +} + +export type A = { + set value({ a, b, c }: { a: string; b: string; c: string }); + get value(); +}; + +export interface I { + set value(_); + get value(): string; +} diff --git a/crates/oxc_isolated_declarations/tests/snapshots/signatures.snap b/crates/oxc_isolated_declarations/tests/snapshots/signatures.snap new file mode 100644 index 0000000000000..757b8cc6a6e74 --- /dev/null +++ b/crates/oxc_isolated_declarations/tests/snapshots/signatures.snap @@ -0,0 +1,25 @@ +--- +source: crates/oxc_isolated_declarations/tests/mod.rs +input_file: crates/oxc_isolated_declarations/tests/fixtures/signatures.ts +--- +==================== .D.TS ==================== + +export interface X { + set value(_: string); +} +export type A = { + set value({ a, b, c }: { + a: string; + b: string; + c: string; + }); + get value(): { + a: string; + b: string; + c: string; + }; +}; +export interface I { + set value(_: string); + get value(): string; +}