feat: type check codeblocks in Markdown file with "deno test --doc" (#…

…11421)

cli/fs_util.rs

@@ -111,6 +111,16 @@ pub fn is_supported_ext_fmt(path: &Path) -> bool {
     false
   }
 }
+/// Checks if the path has extension Deno supports.
+/// This function is similar to is_supported_ext but adds additional extensions
+/// supported by `deno test`.
+pub fn is_supported_ext_test(path: &Path) -> bool {
+  if let Some(ext) = get_extension(path) {
+    matches!(ext.as_str(), "ts" | "tsx" | "js" | "jsx" | "mjs" | "md")
+  } else {
+    false
+  }
+}
 
 /// Get the extension of a file in lowercase.
 pub fn get_extension(file_path: &Path) -> Option<String> {

cli/main.rs

@@ -1030,7 +1030,7 @@ async fn test_command(
         test_runner::collect_test_module_specifiers(
           include.clone(),
           &cwd,
-          fs_util::is_supported_ext,
+          fs_util::is_supported_ext_test,
         )
       } else {
         test_runner::collect_test_module_specifiers(
@@ -1172,7 +1172,7 @@ async fn test_command(
           test_runner::collect_test_module_specifiers(
             include.clone(),
             &cwd,
-            fs_util::is_supported_ext,
+            fs_util::is_supported_ext_test,
           )?
         } else {
           Vec::new()
@@ -1222,7 +1222,7 @@ async fn test_command(
       test_runner::collect_test_module_specifiers(
         include.clone(),
         &cwd,
-        fs_util::is_supported_ext,
+        fs_util::is_supported_ext_test,
       )?
     } else {
       Vec::new()

cli/tests/integration/test_tests.rs

@@ -55,6 +55,12 @@ itest!(doc {
   output: "test/doc.out",
 });
 
+itest!(doc_markdown {
+  args: "test --doc --allow-all test/doc_markdown",
+  exit_code: 1,
+  output: "test/doc_markdown.out",
+});
+
 itest!(quiet {
   args: "test --quiet test/quiet.ts",
   exit_code: 0,

cli/tests/test/doc_markdown.out

@@ -0,0 +1,7 @@
+Check [WILDCARD]/test/doc_markdown/doc.md$11-14.js
+Check [WILDCARD]/test/doc_markdown/doc.md$17-20.ts
+Check [WILDCARD]/test/doc_markdown/doc.md$23-26.ts
+error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'.
+const a: string = 42;
+      ^
+    at [WILDCARD]/test/doc_markdown/doc.md$23-26.ts:1:7

cli/tests/test/doc_markdown/doc.md

@@ -0,0 +1,25 @@
+# Documentation
+
+The following block does not have a language attribute and should be ignored:
+
+```
+This is a fenced block without attributes, it's invalid and it should be ignored.
+```
+
+The following block should be given a js extension on extraction:
+
+```js
+console.log("js");
+```
+
+The following block should be given a ts extension on extraction:
+
+```ts
+console.log("ts");
+```
+
+The following example will trigger the type-checker to fail:
+
+```ts
+const a: string = 42;
+```

cli/tools/test_runner.rs

@@ -1,6 +1,7 @@
 // Copyright 2018-2021 the Deno authors. All rights reserved. MIT license.
 
 use crate::ast;
+use crate::ast::Location;
 use crate::colors;
 use crate::create_main_worker;
 use crate::file_fetcher::File;
@@ -346,6 +347,172 @@ pub async fn run_test_file(
   Ok(())
 }
 
+fn extract_files_from_regex_blocks(
+  location: &Location,
+  source: &str,
+  media_type: &MediaType,
+  blocks_regex: &Regex,
+  lines_regex: &Regex,
+) -> Result<Vec<File>, AnyError> {
+  let files = blocks_regex
+    .captures_iter(&source)
+    .filter_map(|block| {
+      let maybe_attributes = block
+        .get(1)
+        .map(|attributes| attributes.as_str().split(' '));
+
+      let file_media_type = if let Some(mut attributes) = maybe_attributes {
+        match attributes.next() {
+          Some("js") => MediaType::JavaScript,
+          Some("jsx") => MediaType::Jsx,
+          Some("ts") => MediaType::TypeScript,
+          Some("tsx") => MediaType::Tsx,
+          Some("") => *media_type,
+          _ => MediaType::Unknown,
+        }
+      } else {
+        *media_type
+      };
+
+      if file_media_type == MediaType::Unknown {
+        return None;
+      }
+
+      let line_offset = source[0..block.get(0).unwrap().start()]
+        .chars()
+        .filter(|c| *c == '\n')
+        .count();
+
+      let line_count = block.get(0).unwrap().as_str().split('\n').count();
+
+      let body = block.get(2).unwrap();
+      let text = body.as_str();
+
+      // TODO(caspervonb) generate an inline source map
+      let mut file_source = String::new();
+      for line in lines_regex.captures_iter(&text) {
+        let text = line.get(1).unwrap();
+        file_source.push_str(&format!("{}\n", text.as_str()));
+      }
+
+      file_source.push_str("export {};");
+
+      let file_specifier = deno_core::resolve_url_or_path(&format!(
+        "{}${}-{}{}",
+        location.filename,
+        location.line + line_offset,
+        location.line + line_offset + line_count,
+        file_media_type.as_ts_extension(),
+      ))
+      .unwrap();
+
+      Some(File {
+        local: file_specifier.to_file_path().unwrap(),
+        maybe_types: None,
+        media_type: file_media_type,
+        source: file_source,
+        specifier: file_specifier,
+      })
+    })
+    .collect();
+
+  Ok(files)
+}
+
+fn extract_files_from_source_comments(
+  specifier: &ModuleSpecifier,
+  source: &str,
+  media_type: &MediaType,
+) -> Result<Vec<File>, AnyError> {
+  let parsed_module = ast::parse(&specifier.as_str(), &source, &media_type)?;
+  let mut comments = parsed_module.get_comments();
+  comments
+    .sort_by_key(|comment| parsed_module.get_location(&comment.span).line);
+
+  let blocks_regex = Regex::new(r"```([^\n]*)\n([\S\s]*?)```")?;
+  let lines_regex = Regex::new(r"(?:\* ?)(?:\# ?)?(.*)")?;
+
+  let files = comments
+    .iter()
+    .filter(|comment| {
+      if comment.kind != CommentKind::Block || !comment.text.starts_with('*') {
+        return false;
+      }
+
+      true
+    })
+    .flat_map(|comment| {
+      let location = parsed_module.get_location(&comment.span);
+
+      extract_files_from_regex_blocks(
+        &location,
+        &comment.text,
+        &media_type,
+        &blocks_regex,
+        &lines_regex,
+      )
+    })
+    .flatten()
+    .collect();
+
+  Ok(files)
+}
+
+fn extract_files_from_fenced_blocks(
+  specifier: &ModuleSpecifier,
+  source: &str,
+  media_type: &MediaType,
+) -> Result<Vec<File>, AnyError> {
+  let location = Location {
+    filename: specifier.to_string(),
+    line: 1,
+    col: 0,
+  };
+
+  let blocks_regex = Regex::new(r"```([^\n]*)\n([\S\s]*?)```")?;
+  let lines_regex = Regex::new(r"(?:\# ?)?(.*)")?;
+
+  extract_files_from_regex_blocks(
+    &location,
+    &source,
+    &media_type,
+    &blocks_regex,
+    &lines_regex,
+  )
+}
+
+async fn fetch_inline_files(
+  program_state: Arc<ProgramState>,
+  specifiers: Vec<ModuleSpecifier>,
+) -> Result<Vec<File>, AnyError> {
+  let mut files = Vec::new();
+  for specifier in specifiers {
+    let mut fetch_permissions = Permissions::allow_all();
+    let file = program_state
+      .file_fetcher
+      .fetch(&specifier, &mut fetch_permissions)
+      .await?;
+
+    let inline_files = if file.media_type == MediaType::Unknown {
+      extract_files_from_fenced_blocks(
+        &file.specifier,
+        &file.source,
+        &file.media_type,
+      )
+    } else {
+      extract_files_from_source_comments(
+        &file.specifier,
+        &file.source,
+        &file.media_type,
+      )
+    };
+
+    files.extend(inline_files?);
+  }
+
+  Ok(files)
+}
+
 /// Runs tests.
 ///
 #[allow(clippy::too_many_arguments)]
@@ -378,95 +545,16 @@ pub async fn run_tests(
   };
 
   if !doc_modules.is_empty() {
-    let mut test_programs = Vec::new();
-
-    let blocks_regex = Regex::new(r"```([^\n]*)\n([\S\s]*?)```")?;
-    let lines_regex = Regex::new(r"(?:\* ?)(?:\# ?)?(.*)")?;
-
-    for specifier in &doc_modules {
-      let mut fetch_permissions = Permissions::allow_all();
-      let file = program_state
-        .file_fetcher
-        .fetch(&specifier, &mut fetch_permissions)
-        .await?;
+    let files = fetch_inline_files(program_state.clone(), doc_modules).await?;
+    let specifiers = files.iter().map(|file| file.specifier.clone()).collect();
 
-      let parsed_module =
-        ast::parse(&file.specifier.as_str(), &file.source, &file.media_type)?;
-
-      let mut comments = parsed_module.get_comments();
-      comments.sort_by_key(|comment| {
-        let location = parsed_module.get_location(&comment.span);
-        location.line
-      });
-
-      for comment in comments {
-        if comment.kind != CommentKind::Block || !comment.text.starts_with('*')
-        {
-          continue;
-        }
-
-        for block in blocks_regex.captures_iter(&comment.text) {
-          let maybe_attributes = block.get(1).map(|m| m.as_str().split(' '));
-          let media_type = if let Some(mut attributes) = maybe_attributes {
-            match attributes.next() {
-              Some("js") => MediaType::JavaScript,
-              Some("jsx") => MediaType::Jsx,
-              Some("ts") => MediaType::TypeScript,
-              Some("tsx") => MediaType::Tsx,
-              Some("") => file.media_type,
-              _ => MediaType::Unknown,
-            }
-          } else {
-            file.media_type
-          };
-
-          if media_type == MediaType::Unknown {
-            continue;
-          }
-
-          let body = block.get(2).unwrap();
-          let text = body.as_str();
-
-          // TODO(caspervonb) generate an inline source map
-          let mut source = String::new();
-          for line in lines_regex.captures_iter(&text) {
-            let text = line.get(1).unwrap();
-            source.push_str(&format!("{}\n", text.as_str()));
-          }
-
-          source.push_str("export {};");
-
-          let element = block.get(0).unwrap();
-          let span = comment
-            .span
-            .from_inner_byte_pos(element.start(), element.end());
-          let location = parsed_module.get_location(&span);
-
-          let specifier = deno_core::resolve_url_or_path(&format!(
-            "{}${}-{}{}",
-            location.filename,
-            location.line,
-            location.line + element.as_str().split('\n').count(),
-            media_type.as_ts_extension(),
-          ))?;
-
-          let file = File {
-            local: specifier.to_file_path().unwrap(),
-            maybe_types: None,
-            media_type,
-            source: source.clone(),
-            specifier: specifier.clone(),
-          };
-
-          program_state.file_fetcher.insert_cached(file.clone());
-          test_programs.push(file.specifier.clone());
-        }
-      }
+    for file in files {
+      program_state.file_fetcher.insert_cached(file);
     }
 
     program_state
       .prepare_module_graph(
-        test_programs.clone(),
+        specifiers,
         lib.clone(),
         Permissions::allow_all(),
         permissions.clone(),