Skip to content

Commit

Permalink
fix(isolated-declarations): false positive for setter method in `inte…
Browse files Browse the repository at this point in the history
…rface` (#5681)

close: #5668
  • Loading branch information
Dunqing committed Sep 10, 2024
1 parent 6e8409a commit b9bf544
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 12 deletions.
43 changes: 31 additions & 12 deletions crates/oxc_isolated_declarations/src/declaration.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions crates/oxc_isolated_declarations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ mod literal;
mod module;
mod return_type;
mod scope;
mod signatures;
mod types;

use std::{cell::RefCell, collections::VecDeque, mem};
Expand Down
52 changes: 52 additions & 0 deletions crates/oxc_isolated_declarations/src/signatures.rs
Original file line number Diff line number Diff line change
@@ -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>>) {
// <name, (requires_inference, first_param_annotation, return_type)>
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));
}
}
}
}
13 changes: 13 additions & 0 deletions crates/oxc_isolated_declarations/tests/fixtures/signatures.ts
Original file line number Diff line number Diff line change
@@ -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;
}
25 changes: 25 additions & 0 deletions crates/oxc_isolated_declarations/tests/snapshots/signatures.snap
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit b9bf544

Please sign in to comment.