scripts/markdown/check-markdown.js (5)

7-9: Simplify the CONCURRENCY_LIMIT assignment formatting

The ternary operator can be formatted on a single line for better readability.

Apply this diff to adjust the formatting:

-const CONCURRENCY_LIMIT = process.env.CONCURRENCY_LIMIT 
-  ? parseInt(process.env.CONCURRENCY_LIMIT, 10) 
-  : 10;
+const CONCURRENCY_LIMIT = process.env.CONCURRENCY_LIMIT ? parseInt(process.env.CONCURRENCY_LIMIT, 10) : 10;

🧰 Tools

🪛 eslint

[error] 7-9: Replace ⏎··?·parseInt(process.env.CONCURRENCY_LIMIT,·10)·⏎· with ?·parseInt(process.env.CONCURRENCY_LIMIT,·10)

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 7-7: scripts/markdown/check-markdown.js#L7
Added line #L7 was not covered by tests

---

Line range hint 109-148: Remove unnecessary outer try/catch block

The try/catch block from lines 109 to 148 catches filesError only to rethrow it without additional handling, which is unnecessary. Removing it simplifies the code without affecting functionality.

Apply this diff to remove the redundant try/catch block:

-    try {
         // Read directory contents
         const files = await fs.readdir(folderPath);

         // Process files with concurrency control
         const filePromises = files.map(file => limit(async () => {
             const filePath = path.join(folderPath, file);
             const relativeFilePath = path.join(relativePath, file);

             // Skip the folder 'docs/reference/specification'
             if (relativeFilePath.includes('reference/specification')) {
                 return;
             }

             try {
                 // Get file stats
                 const stats = await fs.stat(filePath);

                 // Recurse if directory, otherwise validate markdown file
                 if (stats.isDirectory()) {
                     await checkMarkdownFiles(filePath, validateFunction, relativeFilePath);
                 } else if (path.extname(file) === '.md') {
                     const fileContent = await fs.readFile(filePath, 'utf-8');
                     const { data: frontmatter } = matter(fileContent);

                     const errors = validateFunction(frontmatter);
                     if (errors) {
                         process.exitCode = 1;
                     }
                 }
             } catch (statError) {
                 throw statError;
             }
         }));
         
         // Wait for all file promises to complete
         await Promise.all(filePromises);
-    } catch (filesError) {
-        throw filesError;
-    }

🧰 Tools

🪛 Biome (1.9.4)

[error] 140-140: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

---

[error] 147-147: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

🪛 eslint

[error] 120-120: Replace ················ with ··········

(prettier/prettier)

---

[error] 121-121: Delete ····

(prettier/prettier)

---

[error] 123-123: Delete ····

(prettier/prettier)

---

[error] 123-141: Unnecessary try/catch wrapper.

(no-useless-catch)

---

[error] 124-124: Replace ················ with ··········

(prettier/prettier)

---

[error] 125-125: Delete ······

(prettier/prettier)

---

[error] 127-127: Replace ················ with ··········

(prettier/prettier)

---

[error] 128-128: Delete ······

(prettier/prettier)

---

[error] 129-129: Replace ···················· with ············

(prettier/prettier)

---

[error] 130-130: Delete ······

(prettier/prettier)

---

[error] 131-131: Replace ···················· with ············

(prettier/prettier)

---

[error] 132-132: Replace ···················· with ············

(prettier/prettier)

---

[error] 134-134: Delete ········

(prettier/prettier)

---

[error] 135-135: Replace ···················· with ············

(prettier/prettier)

---

[error] 136-136: Replace ························ with ··············

(prettier/prettier)

---

[error] 137-137: Delete ········

(prettier/prettier)

---

[error] 138-138: Replace ················ with ··········

(prettier/prettier)

---

[error] 139-139: Delete ····

(prettier/prettier)

---

[error] 140-140: Delete ······

(prettier/prettier)

---

[error] 141-141: Delete ····

(prettier/prettier)

---

[error] 142-143: Replace ··}));⏎ with })

(prettier/prettier)

---

[error] 144-144: Insert );⏎⏎

(prettier/prettier)

---

[error] 145-145: Delete ····

(prettier/prettier)

---

[error] 146-146: Delete ··

(prettier/prettier)

---

[error] 147-147: Delete ····

(prettier/prettier)

---

[error] 148-148: Delete ··

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 123-123: scripts/markdown/check-markdown.js#L123
Added line #L123 was not covered by tests

---

[warning] 125-125: scripts/markdown/check-markdown.js#L125
Added line #L125 was not covered by tests

---

[warning] 129-129: scripts/markdown/check-markdown.js#L129
Added line #L129 was not covered by tests

---

[warning] 131-131: scripts/markdown/check-markdown.js#L131
Added line #L131 was not covered by tests

---

[warning] 140-140: scripts/markdown/check-markdown.js#L140
Added line #L140 was not covered by tests

---

[warning] 145-145: scripts/markdown/check-markdown.js#L145
Added line #L145 was not covered by tests

---

[warning] 147-147: scripts/markdown/check-markdown.js#L147
Added line #L147 was not covered by tests

---

Line range hint 105-145: Re-evaluate asynchronous file operations in recursive processing

Based on previous learnings:

"In scripts/markdown/check-markdown.js, when processing large markdown files with recursion and loops, synchronous file reads (fs.readFileSync) are preferred over asynchronous reads."

While the introduction of concurrency control with p-limit mitigates the risk of resource exhaustion, consider whether using synchronous file operations could further simplify the recursion and handling of file descriptors.

Let me know if you would like assistance in refactoring the code to use synchronous file operations.

🧰 Tools

🪛 Biome (1.9.4)

[error] 140-140: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

---

[error] 147-147: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

🪛 eslint

[error] 120-120: Replace ················ with ··········

(prettier/prettier)

---

[error] 121-121: Delete ····

(prettier/prettier)

---

[error] 123-123: Delete ····

(prettier/prettier)

---

[error] 123-141: Unnecessary try/catch wrapper.

(no-useless-catch)

---

[error] 124-124: Replace ················ with ··········

(prettier/prettier)

---

[error] 125-125: Delete ······

(prettier/prettier)

---

[error] 127-127: Replace ················ with ··········

(prettier/prettier)

---

[error] 128-128: Delete ······

(prettier/prettier)

---

[error] 129-129: Replace ···················· with ············

(prettier/prettier)

---

[error] 130-130: Delete ······

(prettier/prettier)

---

[error] 131-131: Replace ···················· with ············

(prettier/prettier)

---

[error] 132-132: Replace ···················· with ············

(prettier/prettier)

---

[error] 134-134: Delete ········

(prettier/prettier)

---

[error] 135-135: Replace ···················· with ············

(prettier/prettier)

---

[error] 136-136: Replace ························ with ··············

(prettier/prettier)

---

[error] 137-137: Delete ········

(prettier/prettier)

---

[error] 138-138: Replace ················ with ··········

(prettier/prettier)

---

[error] 139-139: Delete ····

(prettier/prettier)

---

[error] 140-140: Delete ······

(prettier/prettier)

---

[error] 141-141: Delete ····

(prettier/prettier)

---

[error] 142-143: Replace ··}));⏎ with })

(prettier/prettier)

---

[error] 144-144: Insert );⏎⏎

(prettier/prettier)

---

[error] 145-145: Delete ····

(prettier/prettier)

---

[error] 146-146: Delete ··

(prettier/prettier)

---

[error] 147-147: Delete ····

(prettier/prettier)

---

[error] 148-148: Delete ··

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 123-123: scripts/markdown/check-markdown.js#L123
Added line #L123 was not covered by tests

---

[warning] 125-125: scripts/markdown/check-markdown.js#L125
Added line #L125 was not covered by tests

---

[warning] 129-129: scripts/markdown/check-markdown.js#L129
Added line #L129 was not covered by tests

---

[warning] 131-131: scripts/markdown/check-markdown.js#L131
Added line #L131 was not covered by tests

---

[warning] 140-140: scripts/markdown/check-markdown.js#L140
Added line #L140 was not covered by tests

---

[warning] 145-145: scripts/markdown/check-markdown.js#L145
Added line #L145 was not covered by tests

---

[warning] 147-147: scripts/markdown/check-markdown.js#L147
Added line #L147 was not covered by tests

---

Line range hint 105-148: Add unit tests for asynchronous processing and concurrency control

The new asynchronous logic and concurrency control are significant changes. Please add unit tests to cover these new code paths to maintain code coverage and ensure correct behavior.

🧰 Tools

🪛 Biome (1.9.4)

[error] 140-140: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

---

[error] 147-147: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

🪛 eslint

[error] 120-120: Replace ················ with ··········

(prettier/prettier)

---

[error] 121-121: Delete ····

(prettier/prettier)

---

[error] 123-123: Delete ····

(prettier/prettier)

---

[error] 123-141: Unnecessary try/catch wrapper.

(no-useless-catch)

---

[error] 124-124: Replace ················ with ··········

(prettier/prettier)

---

[error] 125-125: Delete ······

(prettier/prettier)

---

[error] 127-127: Replace ················ with ··········

(prettier/prettier)

---

[error] 128-128: Delete ······

(prettier/prettier)

---

[error] 129-129: Replace ···················· with ············

(prettier/prettier)

---

[error] 130-130: Delete ······

(prettier/prettier)

---

[error] 131-131: Replace ···················· with ············

(prettier/prettier)

---

[error] 132-132: Replace ···················· with ············

(prettier/prettier)

---

[error] 134-134: Delete ········

(prettier/prettier)

---

[error] 135-135: Replace ···················· with ············

(prettier/prettier)

---

[error] 136-136: Replace ························ with ··············

(prettier/prettier)

---

[error] 137-137: Delete ········

(prettier/prettier)

---

[error] 138-138: Replace ················ with ··········

(prettier/prettier)

---

[error] 139-139: Delete ····

(prettier/prettier)

---

[error] 140-140: Delete ······

(prettier/prettier)

---

[error] 141-141: Delete ····

(prettier/prettier)

---

[error] 142-143: Replace ··}));⏎ with })

(prettier/prettier)

---

[error] 144-144: Insert );⏎⏎

(prettier/prettier)

---

[error] 145-145: Delete ····

(prettier/prettier)

---

[error] 146-146: Delete ··

(prettier/prettier)

---

[error] 147-147: Delete ····

(prettier/prettier)

---

[error] 148-148: Delete ··

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 123-123: scripts/markdown/check-markdown.js#L123
Added line #L123 was not covered by tests

---

[warning] 125-125: scripts/markdown/check-markdown.js#L125
Added line #L125 was not covered by tests

---

[warning] 129-129: scripts/markdown/check-markdown.js#L129
Added line #L129 was not covered by tests

---

[warning] 131-131: scripts/markdown/check-markdown.js#L131
Added line #L131 was not covered by tests

---

[warning] 140-140: scripts/markdown/check-markdown.js#L140
Added line #L140 was not covered by tests

---

[warning] 145-145: scripts/markdown/check-markdown.js#L145
Added line #L145 was not covered by tests

---

[warning] 147-147: scripts/markdown/check-markdown.js#L147
Added line #L147 was not covered by tests

---

Line range hint 7-148: Adjust code formatting to meet style guidelines

Several formatting inconsistencies are detected by the linter, primarily related to indentation and line breaks. Running the project's code formatter (e.g., Prettier) will resolve these issues.

🧰 Tools

🪛 Biome (1.9.4)

[error] 140-140: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

---

[error] 147-147: The catch clause that only rethrows the original error is useless.

An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.

(lint/complexity/noUselessCatch)

🪛 eslint

[error] 120-120: Replace ················ with ··········

(prettier/prettier)

---

[error] 121-121: Delete ····

(prettier/prettier)

---

[error] 123-123: Delete ····

(prettier/prettier)

---

[error] 123-141: Unnecessary try/catch wrapper.

(no-useless-catch)

---

[error] 124-124: Replace ················ with ··········

(prettier/prettier)

---

[error] 125-125: Delete ······

(prettier/prettier)

---

[error] 127-127: Replace ················ with ··········

(prettier/prettier)

---

[error] 128-128: Delete ······

(prettier/prettier)

---

[error] 129-129: Replace ···················· with ············

(prettier/prettier)

---

[error] 130-130: Delete ······

(prettier/prettier)

---

[error] 131-131: Replace ···················· with ············

(prettier/prettier)

---

[error] 132-132: Replace ···················· with ············

(prettier/prettier)

---

[error] 134-134: Delete ········

(prettier/prettier)

---

[error] 135-135: Replace ···················· with ············

(prettier/prettier)

---

[error] 136-136: Replace ························ with ··············

(prettier/prettier)

---

[error] 137-137: Delete ········

(prettier/prettier)

---

[error] 138-138: Replace ················ with ··········

(prettier/prettier)

---

[error] 139-139: Delete ····

(prettier/prettier)

---

[error] 140-140: Delete ······

(prettier/prettier)

---

[error] 141-141: Delete ····

(prettier/prettier)

---

[error] 142-143: Replace ··}));⏎ with })

(prettier/prettier)

---

[error] 144-144: Insert );⏎⏎

(prettier/prettier)

---

[error] 145-145: Delete ····

(prettier/prettier)

---

[error] 146-146: Delete ··

(prettier/prettier)

---

[error] 147-147: Delete ····

(prettier/prettier)

---

[error] 148-148: Delete ··

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 123-123: scripts/markdown/check-markdown.js#L123
Added line #L123 was not covered by tests

---

[warning] 125-125: scripts/markdown/check-markdown.js#L125
Added line #L125 was not covered by tests

---

[warning] 129-129: scripts/markdown/check-markdown.js#L129
Added line #L129 was not covered by tests

---

[warning] 131-131: scripts/markdown/check-markdown.js#L131
Added line #L131 was not covered by tests

---

[warning] 140-140: scripts/markdown/check-markdown.js#L140
Added line #L140 was not covered by tests

---

[warning] 145-145: scripts/markdown/check-markdown.js#L145
Added line #L145 was not covered by tests

---

[warning] 147-147: scripts/markdown/check-markdown.js#L147
Added line #L147 was not covered by tests

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-11-12T06:59:33.851Z
Learning: In `scripts/markdown/check-markdown.js`, when processing large markdown files with recursion and loops, synchronous file reads (`fs.readFileSync`) are preferred over asynchronous reads.

scripts/markdown/check-markdown.js (3)

6-7: Consider adding validation for CONCURRENCY_LIMIT.

While the default of 10 is reasonable, the environment variable should be validated to ensure it's a positive integer.

 // Configuration
-const CONCURRENCY_LIMIT = process.env.CONCURRENCY_LIMIT ? parseInt(process.env.CONCURRENCY_LIMIT, 10) : 10;
+const parsedLimit = process.env.CONCURRENCY_LIMIT ? parseInt(process.env.CONCURRENCY_LIMIT, 10) : 10;
+const CONCURRENCY_LIMIT = Number.isInteger(parsedLimit) && parsedLimit > 0 ? parsedLimit : 10;

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 7-7: scripts/markdown/check-markdown.js#L7
Added line #L7 was not covered by tests

---

15-20: Avoid side effects in URL validation.

The current implementation creates a new URL object just for validation. Consider using the URL constructor's return value instead.

 function isValidURL(str) {
   try {
-    new URL(str);
-    return true;
+    return !!new URL(str);
   } catch (err) {
     return false;
   }
 }

🧰 Tools

🪛 eslint

[error] 16-16: Do not use 'new' for side effects.

(no-new)

🪛 GitHub Check: codecov/patch

[warning] 15-17: scripts/markdown/check-markdown.js#L15-L17
Added lines #L15 - L17 were not covered by tests

---

[warning] 19-19: scripts/markdown/check-markdown.js#L19
Added line #L19 was not covered by tests

---

135-136: Document the reason for skipping 'reference/specification'.

The code skips the 'reference/specification' directory without explanation. Consider adding a comment explaining why this directory is excluded.

         // Skip specific folder
+        // Skip reference/specification as it contains auto-generated markdown files
         if (relativeFilePath.includes('reference/specification')) {

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 135-136: scripts/markdown/check-markdown.js#L135-L136
Added lines #L135 - L136 were not covered by tests

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-11-12T06:59:33.851Z
Learning: In `scripts/markdown/check-markdown.js`, when processing large markdown files with recursion and loops, synchronous file reads (`fs.readFileSync`) are preferred over asynchronous reads.

scripts/markdown/check-markdown.js (2)

111-111: Consider batching file operations for better performance

The current implementation processes files one by one with concurrency control. Consider batching file operations to reduce overhead:

-const filePromises = files.map((file) =>
-    limit(async () => {
-        const filePath = path.join(folderPath, file);
-        const relativeFilePath = path.join(relativePath, file);
+const batchSize = 5;
+const filePromises = [];
+for (let i = 0; i < files.length; i += batchSize) {
+    const batch = files.slice(i, i + batchSize);
+    filePromises.push(
+        limit(async () => {
+            const results = await Promise.all(
+                batch.map(async (file) => {
+                    const filePath = path.join(folderPath, file);
+                    const relativeFilePath = path.join(relativePath, file);

Also applies to: 119-122

🧰 Tools

🪛 eslint

[error] 111-111: Delete ··

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 111-112: scripts/markdown/check-markdown.js#L111-L112
Added lines #L111 - L112 were not covered by tests

---

125-126: Document the reference/specification exclusion

The code skips the 'reference/specification' folder without explanation. Consider adding a comment explaining why this folder is excluded.

+// Skip auto-generated specification documentation
 if (relativeFilePath.includes('reference/specification')) {
     return [];
 }

🧰 Tools

🪛 eslint

[error] 125-125: Replace ················ with ········

(prettier/prettier)

---

[error] 126-126: Replace ···················· with ··········

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 125-126: scripts/markdown/check-markdown.js#L125-L126
Added lines #L125 - L126 were not covered by tests

Configuration used: .coderabbit.yaml
Review profile: CHILL

Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:31-31
Timestamp: 2024-11-12T06:59:33.852Z
Learning: In `scripts/markdown/check-markdown.js`, the `frontmatter` object cannot have a property named `hasOwnProperty`, so using `frontmatter.hasOwnProperty(attr)` is acceptable.

Learnt from: anshgoyalevil
PR: asyncapi/website#3301
File: scripts/markdown/check-markdown.js:0-0
Timestamp: 2024-11-12T06:59:33.851Z
Learning: In `scripts/markdown/check-markdown.js`, when processing large markdown files with recursion and loops, synchronous file reads (`fs.readFileSync`) are preferred over asynchronous reads.

scripts/markdown/check-markdown.js (3)

7-11: Consider adding input validation for CONCURRENCY_LIMIT

While the code handles non-integer and negative values, it should also handle:

• Very large numbers that could cause performance issues
• Non-numeric strings that would result in NaN

 const parsedLimit = process.env.CONCURRENCY_LIMIT
   ? parseInt(process.env.CONCURRENCY_LIMIT, 10)
   : 10;
 const CONCURRENCY_LIMIT =
-  Number.isInteger(parsedLimit) && parsedLimit > 0 ? parsedLimit : 10;
+  Number.isInteger(parsedLimit) && parsedLimit > 0 && parsedLimit <= 100 ? parsedLimit : 10;

🧰 Tools

🪛 eslint

[error] 7-9: Replace ⏎··?·parseInt(process.env.CONCURRENCY_LIMIT,·10)⏎· with ·?·parseInt(process.env.CONCURRENCY_LIMIT,·10)

(prettier/prettier)

---

[error] 10-11: Delete ⏎·

(prettier/prettier)

🪛 GitHub Check: codecov/patch

[warning] 7-7: scripts/markdown/check-markdown.js#L7
Added line #L7 was not covered by tests

---

[warning] 11-11: scripts/markdown/check-markdown.js#L11
Added line #L11 was not covered by tests

---

19-24: Optimize URL validation to avoid object creation

The current implementation creates a new URL object even when validation succeeds. Consider using URL.canParse() for better performance.

 function isValidURL(str) {
-  try {
-    new URL(str);
-    return true;
-  } catch (err) {
-    return false;
-  }
+  return URL.canParse(str);
 }

🧰 Tools

🪛 eslint

[error] 20-20: Do not use 'new' for side effects.

(no-new)

🪛 GitHub Check: codecov/patch

[warning] 19-21: scripts/markdown/check-markdown.js#L19-L21
Added lines #L19 - L21 were not covered by tests

---

[warning] 23-23: scripts/markdown/check-markdown.js#L23
Added line #L23 was not covered by tests

---

87-87: Return empty array instead of null for consistency

Both validation functions return null when there are no errors, but returning an empty array would be more consistent and avoid null checks.

-  return errors.length ? errors : null;
+  return errors;

Also applies to: 112-112

🧰 Tools

🪛 GitHub Check: codecov/patch

[warning] 87-87: scripts/markdown/check-markdown.js#L87
Added line #L87 was not covered by tests

Configuration used: .coderabbit.yaml
Review profile: CHILL

-    if (!frontmatter.hasOwnProperty(attr)) {
+    if (!Object.hasOwn(frontmatter, attr)) {