Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_parser): Allow arguments in d.ts files
Browse files Browse the repository at this point in the history
Ambient context has slightly different rules than parsing in a normal context. One such difference is that `arguments` and future reserved keywords are valid identifiers.

Our parser already correctly handled this case by setting the `strict_mode` to `None` when entering an ambient context. However, we initialized `strict_mode` with `Module` for `d.ts` files.

This PR correctly initializes the parser state with `strict = None` if the file is a type script definition file.

## Tests

I added a new parser test and verified that the `jquery.d.ts` can now be formatted

```bash
cargo run --bin rome format ../vscode/extensions/html-language-features/server/lib/jquery.d.ts --write
```
  • Loading branch information
MichaReiser committed Oct 11, 2022
1 parent 2ffe2b7 commit 92b39d7
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 30 deletions.
28 changes: 15 additions & 13 deletions crates/rome_js_parser/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::Parser;
use bitflags::bitflags;
use indexmap::IndexMap;
use rome_js_syntax::{Language, SourceType};
use rome_js_syntax::SourceType;
use rome_rowan::{TextRange, TextSize};
use std::collections::HashSet;
use std::ops::{Deref, DerefMut, Range};
Expand Down Expand Up @@ -99,31 +99,33 @@ pub(crate) enum StrictMode {

impl ParserState {
pub fn new(source_type: &SourceType) -> Self {
let is_definition_file = source_type.language().is_definition_file();
let is_module = source_type.module_kind().is_module();

// test d.ts arguments_in_definition_file
// function a(...arguments: any[]): void;
let strict = if is_module && !is_definition_file {
Some(StrictMode::Module)
} else {
None
};

let mut state = ParserState {
parsing_context: ParsingContextFlags::TOP_LEVEL,
label_set: IndexMap::new(),
strict: if source_type.module_kind().is_module() {
Some(StrictMode::Module)
} else {
None
},
strict,
default_item: None,
name_map: IndexMap::new(),
duplicate_binding_parent: None,
not_parenthesized_arrow: Default::default(),
speculative_parsing: false,
};

if source_type.module_kind().is_module() {
if is_module {
state.parsing_context |= ParsingContextFlags::IN_ASYNC
}

if matches!(
source_type.language(),
Language::TypeScript {
definition_file: true
}
) {
if is_definition_file {
state.parsing_context |= ParsingContextFlags::AMBIENT_CONTEXT
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
JsModule {
interpreter_token: missing (optional),
directives: JsDirectiveList [],
items: JsModuleItemList [
TsDeclareFunctionDeclaration {
async_token: missing (optional),
function_token: FUNCTION_KW@0..9 "function" [] [Whitespace(" ")],
id: JsIdentifierBinding {
name_token: IDENT@9..10 "a" [] [],
},
type_parameters: missing (optional),
parameters: JsParameters {
l_paren_token: L_PAREN@10..11 "(" [] [],
items: JsParameterList [
JsRestParameter {
dotdotdot_token: DOT3@11..14 "..." [] [],
binding: JsIdentifierBinding {
name_token: IDENT@14..23 "arguments" [] [],
},
type_annotation: TsTypeAnnotation {
colon_token: COLON@23..25 ":" [] [Whitespace(" ")],
ty: TsArrayType {
element_type: TsAnyType {
any_token: ANY_KW@25..28 "any" [] [],
},
l_brack_token: L_BRACK@28..29 "[" [] [],
r_brack_token: R_BRACK@29..30 "]" [] [],
},
},
},
],
r_paren_token: R_PAREN@30..31 ")" [] [],
},
return_type_annotation: TsReturnTypeAnnotation {
colon_token: COLON@31..33 ":" [] [Whitespace(" ")],
ty: TsVoidType {
void_token: VOID_KW@33..37 "void" [] [],
},
},
semicolon_token: SEMICOLON@37..38 ";" [] [],
},
],
eof_token: EOF@38..39 "" [Newline("\n")] [],
}

0: JS_MODULE@0..39
0: (empty)
1: JS_DIRECTIVE_LIST@0..0
2: JS_MODULE_ITEM_LIST@0..38
0: TS_DECLARE_FUNCTION_DECLARATION@0..38
0: (empty)
1: FUNCTION_KW@0..9 "function" [] [Whitespace(" ")]
2: JS_IDENTIFIER_BINDING@9..10
0: IDENT@9..10 "a" [] []
3: (empty)
4: JS_PARAMETERS@10..31
0: L_PAREN@10..11 "(" [] []
1: JS_PARAMETER_LIST@11..30
0: JS_REST_PARAMETER@11..30
0: DOT3@11..14 "..." [] []
1: JS_IDENTIFIER_BINDING@14..23
0: IDENT@14..23 "arguments" [] []
2: TS_TYPE_ANNOTATION@23..30
0: COLON@23..25 ":" [] [Whitespace(" ")]
1: TS_ARRAY_TYPE@25..30
0: TS_ANY_TYPE@25..28
0: ANY_KW@25..28 "any" [] []
1: L_BRACK@28..29 "[" [] []
2: R_BRACK@29..30 "]" [] []
2: R_PAREN@30..31 ")" [] []
5: TS_RETURN_TYPE_ANNOTATION@31..37
0: COLON@31..33 ":" [] [Whitespace(" ")]
1: TS_VOID_TYPE@33..37
0: VOID_KW@33..37 "void" [] []
6: SEMICOLON@37..38 ";" [] []
3: EOF@38..39 "" [Newline("\n")] []
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
function a(...arguments: any[]): void;
29 changes: 19 additions & 10 deletions crates/rome_js_syntax/src/source_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ pub enum ModuleKind {
}

impl ModuleKind {
pub fn is_script(&self) -> bool {
pub const fn is_script(&self) -> bool {
matches!(self, ModuleKind::Script)
}
pub fn is_module(&self) -> bool {
pub const fn is_module(&self) -> bool {
matches!(self, ModuleKind::Module)
}
}
Expand All @@ -58,10 +58,10 @@ pub enum LanguageVariant {
}

impl LanguageVariant {
pub fn is_standard(&self) -> bool {
pub const fn is_standard(&self) -> bool {
matches!(self, LanguageVariant::Standard)
}
pub fn is_jsx(&self) -> bool {
pub const fn is_jsx(&self) -> bool {
matches!(self, LanguageVariant::Jsx)
}
}
Expand All @@ -77,12 +77,21 @@ pub enum Language {
}

impl Language {
pub fn is_javascript(&self) -> bool {
pub const fn is_javascript(&self) -> bool {
matches!(self, Language::JavaScript)
}
pub fn is_typescript(&self) -> bool {
pub const fn is_typescript(&self) -> bool {
matches!(self, Language::TypeScript { .. })
}

pub const fn is_definition_file(&self) -> bool {
matches!(
self,
Language::TypeScript {
definition_file: true
}
)
}
}

#[derive(Clone, Copy, Debug, Default)]
Expand Down Expand Up @@ -135,17 +144,17 @@ impl SourceType {
}
}

pub fn with_module_kind(mut self, kind: ModuleKind) -> Self {
pub const fn with_module_kind(mut self, kind: ModuleKind) -> Self {
self.module_kind = kind;
self
}

pub fn with_version(mut self, version: LanguageVersion) -> Self {
pub const fn with_version(mut self, version: LanguageVersion) -> Self {
self.version = version;
self
}

pub fn with_variant(mut self, variant: LanguageVariant) -> Self {
pub const fn with_variant(mut self, variant: LanguageVariant) -> Self {
self.variant = variant;
self
}
Expand All @@ -166,7 +175,7 @@ impl SourceType {
self.module_kind
}

pub fn is_module(&self) -> bool {
pub const fn is_module(&self) -> bool {
self.module_kind.is_module()
}
}
Expand Down
20 changes: 13 additions & 7 deletions xtask/codegen/src/parser_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct Test {
enum Language {
JavaScript,
TypeScript,
TypeScriptDefinition,
Jsx,
Tsx,
}
Expand All @@ -112,17 +113,21 @@ impl Language {
match self {
Language::JavaScript => "js",
Language::TypeScript => "ts",
Language::TypeScriptDefinition => "d.ts",
Language::Jsx => "jsx",
Language::Tsx => "tsx",
}
}

fn from_extension(extension: &str) -> Option<Language> {
let language = match extension {
"js" => Language::JavaScript,
"ts" => Language::TypeScript,
"jsx" => Language::Jsx,
"tsx" => Language::Tsx,
fn from_file_name(name: &str) -> Option<Language> {
let language = match name.rsplit_once('.')? {
(_, "js") => Language::JavaScript,
(rest, "ts") => match rest.rsplit_once('.') {
Some((_, "d")) => Language::TypeScriptDefinition,
_ => Language::TypeScript,
},
(_, "jsx") => Language::Jsx,
(_, "tsx") => Language::Tsx,
_ => {
return None;
}
Expand Down Expand Up @@ -153,6 +158,7 @@ fn collect_tests(s: &str) -> Vec<Test> {
Some(("jsx", name)) => (Language::Jsx, name),
Some(("js", name)) => (Language::JavaScript, name),
Some(("ts", name)) => (Language::TypeScript, name),
Some(("d.ts", name)) => (Language::TypeScriptDefinition, name),
Some(("tsx", name)) => (Language::Tsx, name),
Some((name, _)) => (Language::JavaScript, name),
_ => (Language::JavaScript, suffix),
Expand Down Expand Up @@ -212,7 +218,7 @@ fn existing_tests(dir: &Path, ok: bool) -> Result<HashMap<String, (PathBuf, Test
let language = path
.extension()
.and_then(|ext| ext.to_str())
.and_then(Language::from_extension);
.and_then(Language::from_file_name);

if let Some(language) = language {
let name = path
Expand Down

0 comments on commit 92b39d7

Please sign in to comment.