fix(parser): allow illegal return in astro files (biomejs#2273)

unvalley · Apr 2, 2024 · 2c5e309 · 2c5e309
1 parent a6f5f0e
commit 2c5e309
Show file tree

Hide file tree

Showing 9 changed files with 97 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,9 +51,20 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 ### Parser
 
-## 1.6.3 (2024-03-25)
+- The parser doesn't throw any error when the frontmatter of `.astro` files contains an illegal return:
 
-### Analyzer
+  ```astro
+  ---
+  const condition = true;
+  if (condition) {
+    return "Something";
+  }
+  ---
+  <div></div>
+  ```
+  Contributed by @ematipico
+
+## 1.6.3 (2024-03-25)
 
 ### CLI
 
@@ -69,16 +80,12 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 - Complete the documentation and overrides support for options `formatter.lineEnding`, `[language].formatter.lineEnding`, `formatter.attributePosition` and `javascript.formatter.attributePosition`. Contributed by @Sec-ant
 
-### Editors
-
 ### Formatter
 
 #### Bug fixes
 
 - Fix [#2172](https://github.com/biomejs/biome/issues/2172) by breaking long object destructuring patterns. Contributed by @ah-yu
 
-### JavaScript APIs
-
 ### Linter
 
 #### New features
@@ -96,8 +103,6 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 - Quickfix action no longer autofixes lint rule errors on save when `linter` is disabled ([#2161](https://github.com/biomejs/biome/issues/2161)). Contributed by @Sec-ant
 - Range formatting for Astro/Svelte/Vue doesn't place code out of place, especially when formatting on paste is enabled. Contributed by @ematipico
 
-### Parser
-
 ## 1.6.2 (2024-03-22)
 
 ### Analyzer

diff --git a/crates/biome_cli/tests/cases/handle_astro_files.rs b/crates/biome_cli/tests/cases/handle_astro_files.rs
@@ -36,6 +36,15 @@ import { Code } from "astro:components";
 ---
 <div></div>"#;
 
+const ASTRO_RETURN: &str = r#"---
+const foo = true;
+if (foo) {
+    return "Something";
+}
+
+---
+<div></div>"#;
+
 const ASTRO_FILE_IMPORTS_AFTER: &str = r#"---
 import { Code } from "astro:components";
 import { getLocale } from "astro:i18n";
@@ -272,3 +281,28 @@ fn sorts_imports_write() {
         result,
     ));
 }
+
+#[test]
+fn does_not_throw_parse_error_for_return() {
+    let mut fs = MemoryFileSystem::default();
+    let mut console = BufferConsole::default();
+
+    let astro_file_path = Path::new("file.astro");
+    fs.insert(astro_file_path.into(), ASTRO_RETURN.as_bytes());
+
+    let result = run_cli(
+        DynRef::Borrowed(&mut fs),
+        &mut console,
+        Args::from([("lint"), astro_file_path.as_os_str().to_str().unwrap()].as_slice()),
+    );
+
+    assert!(result.is_ok(), "run_cli returned {result:?}");
+
+    assert_cli_snapshot(SnapshotPayload::new(
+        module_path!(),
+        "does_not_throw_parse_error_for_return",
+        fs,
+        console,
+        result,
+    ));
+}
diff --git a/crates/biome_cli/tests/commands/check.rs b/crates/biome_cli/tests/commands/check.rs
@@ -191,7 +191,6 @@ fn maximum_diagnostics() {
         .filter(|m| m.level == LogLevel::Log)
         .any(|m| {
             let content = format!("{:?}", m.content);
-            dbg!(&content);
             content.contains("The number of diagnostics exceeds the number allowed by Biome")
                 && content.contains("Diagnostics not shown")
                 && content.contains("77")

diff --git a/.../tests/snapshots/main_cases_handle_astro_files/does_not_throw_parse_error_for_return.snap b/.../tests/snapshots/main_cases_handle_astro_files/does_not_throw_parse_error_for_return.snap
@@ -0,0 +1,22 @@
+---
+source: crates/biome_cli/tests/snap_test.rs
+expression: content
+---
+## `file.astro`
+
+```astro
+---
+const foo = true;
+if (foo) {
+    return "Something";
+}
+
+---
+<div></div>
+```
+
+# Emitted Messages
+
+```block
+Checked 1 file in <TIME>. No fixes needed.
+```
diff --git a/crates/biome_js_parser/src/syntax/stmt.rs b/crates/biome_js_parser/src/syntax/stmt.rs
@@ -725,7 +725,8 @@ fn parse_return_statement(p: &mut JsParser) -> ParsedSyntax {
     semi(p, TextRange::new(start, p.cur_range().end()));
     let mut complete = m.complete(p, JS_RETURN_STATEMENT);
 
-    if !p.state().in_function() {
+    // The frontmatter of Astro files is executed inside a function during the compilation, so it's safe to have illegal returns
+    if !p.state().in_function() && !p.source_type.as_embedding_kind().is_astro() {
         let err = p.err_builder(
             "Illegal return statement outside of a function",
             complete.range(p),

diff --git a/crates/biome_service/src/file_handlers/astro.rs b/crates/biome_service/src/file_handlers/astro.rs
@@ -96,15 +96,17 @@ impl ExtensionHandler for AstroFileHandler {
 
 fn parse(
     _rome_path: &BiomePath,
-    _file_source: DocumentFileSource,
+    file_source: DocumentFileSource,
     text: &str,
     _settings: SettingsHandle,
     cache: &mut NodeCache,
 ) -> ParseResult {
     let frontmatter = AstroFileHandler::input(text);
     let parse = parse_js_with_cache(
         frontmatter,
-        JsFileSource::ts(),
+        file_source
+            .to_js_file_source()
+            .unwrap_or(JsFileSource::ts()),
         JsParserOptions::default(),
         cache,
     );
@@ -117,7 +119,7 @@ fn parse(
             root.as_send().unwrap(),
             diagnostics,
         ),
-        language: Some(JsFileSource::ts().into()),
+        language: Some(JsFileSource::astro().into()),
     }
 }
 

diff --git a/crates/biome_service/src/file_handlers/svelte.rs b/crates/biome_service/src/file_handlers/svelte.rs
@@ -11,7 +11,7 @@ use crate::WorkspaceError;
 use biome_formatter::Printed;
 use biome_fs::BiomePath;
 use biome_js_parser::{parse_js_with_cache, JsParserOptions};
-use biome_js_syntax::{JsFileSource, TextRange, TextSize};
+use biome_js_syntax::{EmbeddingKind, JsFileSource, TextRange, TextSize};
 use biome_parser::AnyParse;
 use biome_rowan::NodeCache;
 use lazy_static::lazy_static;
@@ -77,7 +77,9 @@ impl SvelteFileHandler {
         matches
             .and_then(|captures| captures.name("lang"))
             .filter(|lang| lang.as_str() == "ts")
-            .map_or(JsFileSource::js_module(), |_| JsFileSource::ts())
+            .map_or(JsFileSource::js_module(), |_| {
+                JsFileSource::ts().with_embedding_kind(EmbeddingKind::Svelte)
+            })
     }
 }
 

diff --git a/crates/biome_service/src/file_handlers/vue.rs b/crates/biome_service/src/file_handlers/vue.rs
@@ -11,7 +11,7 @@ use crate::WorkspaceError;
 use biome_formatter::Printed;
 use biome_fs::BiomePath;
 use biome_js_parser::{parse_js_with_cache, JsParserOptions};
-use biome_js_syntax::{JsFileSource, TextRange, TextSize};
+use biome_js_syntax::{EmbeddingKind, JsFileSource, TextRange, TextSize};
 use biome_parser::AnyParse;
 use biome_rowan::NodeCache;
 use lazy_static::lazy_static;
@@ -77,7 +77,9 @@ impl VueFileHandler {
         matches
             .and_then(|captures| captures.name("lang"))
             .filter(|lang| lang.as_str() == "ts")
-            .map_or(JsFileSource::js_module(), |_| JsFileSource::ts())
+            .map_or(JsFileSource::js_module(), |_| {
+                JsFileSource::ts().with_embedding_kind(EmbeddingKind::Vue)
+            })
     }
 }
 

diff --git a/website/src/content/docs/internals/changelog.md b/website/src/content/docs/internals/changelog.md
@@ -57,9 +57,20 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 ### Parser
 
-## 1.6.3 (2024-03-25)
+- The parser doesn't throw any error when the frontmatter of `.astro` files contains an illegal return:
 
-### Analyzer
+  ```astro
+  ---
+  const condition = true;
+  if (condition) {
+    return "Something";
+  }
+  ---
+  <div></div>
+  ```
+  Contributed by @ematipico
+
+## 1.6.3 (2024-03-25)
 
 ### CLI
 
@@ -75,16 +86,12 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 
 - Complete the documentation and overrides support for options `formatter.lineEnding`, `[language].formatter.lineEnding`, `formatter.attributePosition` and `javascript.formatter.attributePosition`. Contributed by @Sec-ant
 
-### Editors
-
 ### Formatter
 
 #### Bug fixes
 
 - Fix [#2172](https://github.com/biomejs/biome/issues/2172) by breaking long object destructuring patterns. Contributed by @ah-yu
 
-### JavaScript APIs
-
 ### Linter
 
 #### New features
@@ -102,8 +109,6 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
 - Quickfix action no longer autofixes lint rule errors on save when `linter` is disabled ([#2161](https://github.com/biomejs/biome/issues/2161)). Contributed by @Sec-ant
 - Range formatting for Astro/Svelte/Vue doesn't place code out of place, especially when formatting on paste is enabled. Contributed by @ematipico
 
-### Parser
-
 ## 1.6.2 (2024-03-22)
 
 ### Analyzer