preview/ast-grep #65
preview/ast-grep #65
Conversation
WalkthroughThis update introduces new configuration and linting rules for JavaScript and TypeScript codebases, focusing on variable naming and console usage. It also adds new utility functions, a React component, and a TypeScript async function for demonstration. The configuration enables early access features and integrates custom AST-grep rules and packages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant greet.js
participant Console
participant HelloWorld (React)
User->>greet.js: call greet(name, surname)
greet.js->>Console: print greeting message
User->>greet.js: call welcome(name, surname)
greet.js->>Console: print welcome message
User->>HelloWorld (React): render <HelloWorld greeting, greeted, ...>
HelloWorld (React)->>Console: (optional) print based on props
sequenceDiagram
participant Developer
participant AST-grep Linter
participant Source Files
Developer->>AST-grep Linter: run lint rules
AST-grep Linter->>Source Files: scan for console usage and const naming
AST-grep Linter-->>Developer: report violations or suggest fixes
Poem
Impact AnalysisNew greeting utilities and HelloWorld React componentℹ️ Low 🔗 Impacts API 🔄 Impacts behavior Adds a new module exporting console-based greeting functions ( Review prop defaults, styling, and event handling in the Test console output of 🔍 Related Files
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 YAMLlint (1.37.1).coderabbit.yaml[Errno 2] No such file or directory: '.coderabbit.yaml' rules/no-console-except-error.yml[Errno 2] No such file or directory: 'rules/no-console-except-error.yml' rules/replace-unnecessary-type-definition-use-state.yml[Errno 2] No such file or directory: 'rules/replace-unnecessary-type-definition-use-state.yml'
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
.coderabbit.yaml (1)
1-14: Missing ast-grep rule directories in configurationThe directories referenced under
ast-grep.rule_dirsdo not exist in the repo, so your custom rules won’t be loaded:
- “rules/”
- “test-missing-dir/”
Also ensure your custom package is installed or reachable:
test-peter-rabbit/test-ast-grep-custom-packagePlease update your
.coderabbit.yamlor repo structure accordingly:• Create the missing directories and add your AST-Grep rules there
• Or remove/adjust the paths inast-grep.rule_dirs
• Confirm that the packagetest-peter-rabbit/test-ast-grep-custom-packageis available (e.g., installed or in your project’s path)[coderabbit.yaml lines 7–11]
🧹 Nitpick comments (1)
rules/replace-unnecessary-type-definition-use-state.yml (1)
1-8: Effective rule for removing unnecessary TypeScript type annotations.The rule correctly identifies and fixes unnecessary explicit
<string>type parameters inuseStatecalls, allowing TypeScript to infer the type from the initial value.Consider extending this rule to handle other common unnecessary type annotations:
rule: any: - pattern: useState<string>($A) + - pattern: useState<number>($A) + - pattern: useState<boolean>($A)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.coderabbit.yaml(1 hunks)greet.js(1 hunks)jsx_ast_grep_failure.ts(1 hunks)rules/no-console-except-error.yml(1 hunks)rules/replace-unnecessary-type-definition-use-state.yml(1 hunks)rules/ts-const.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts}
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🪛 ast-grep (0.38.6)
jsx_ast_grep_failure.ts
[hint] 11-11:
Context: await (new Promise(() => {console.log("logging this long task")}))
Suggested replacement: (new Promise(() => {console.log("logging this long task")}))
(no-await-in-promise-all-custom)
🪛 Biome (1.9.4)
greet.js
[error] 75-75: Illegal return statement outside of a function
(parse)
[error] 75-76: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🔇 Additional comments (5)
rules/no-console-except-error.yml (1)
1-14: Well-structured ast-grep rule for console usage restrictions.The rule correctly implements the intended behavior:
- Restricts
console.log,console.debug, andconsole.warneverywhere- Allows
console.erroronly inside catch clauses- Uses appropriate pattern matching with constraints and negation logic
The implementation follows ast-grep best practices with clear pattern definitions and proper constraint usage.
rules/ts-const.yaml (1)
3-16: Well-designed rule for enforcing uppercase constants.The rule correctly:
- Matches both typed and untyped const declarations
- Uses regex to identify non-uppercase variable names
- Restricts checking to top-level program scope
- Provides clear warning messages
The regex pattern
.*[^A-Z_].*effectively catches any name containing non-uppercase characters or characters other than underscores.greet.js (3)
58-63: PropTypes definition looks good.The PropTypes are properly defined and cover all the props used in the component.
98-104: Correct console usage in catch block.The function correctly uses
console.errorin the catch block, which is allowed by the AST-grep linting rules.
6-8: Console.log usage violates linting rules.The
console.logstatement violates the AST-grep linting rule that disallowsconsole.log,console.debug, andconsole.warnexcept forconsole.errorinside catch clauses.Consider replacing with
console.errorif this is for error logging, or remove the console statement if it's for debugging purposes:- console.log(`Hello, ${name} ${surname}!`) + console.error(`Hello, ${name} ${surname}!`)Likely an incorrect or invalid review comment.
| @@ -0,0 +1,15 @@ | |||
| id: ts-const | |||
| language: Typescript | |||
There was a problem hiding this comment.
Fix language specification capitalization.
The language should be lowercase "typescript" to match ast-grep's language specifications.
-language: Typescript
+language: typescript📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| language: Typescript | |
| language: typescript |
🤖 Prompt for AI Agents
In rules/ts-const.yaml at line 2, change the language specification from
"Typescript" to lowercase "typescript" to comply with ast-grep's expected
language format.
| @@ -0,0 +1,22 @@ | |||
| import { useState } from 'react' | |||
|
|
|||
| const greeting: string = "Hello, world!"; | |||
There was a problem hiding this comment.
Constant variable name violates coding guidelines.
According to the coding guidelines, all constant variables in JS/TS files must be defined using UPPER CASE letters.
-const greeting: string = "Hello, world!";
+const GREETING: string = "Hello, world!";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const greeting: string = "Hello, world!"; | |
| const GREETING: string = "Hello, world!"; |
🤖 Prompt for AI Agents
In jsx_ast_grep_failure.ts at line 3, the constant variable name 'greeting'
violates the coding guideline requiring constant variables to be in UPPER CASE.
Rename the variable to 'GREETING' to comply with the naming convention.
| const [state, setState] = useState<string>("test string") | ||
|
|
||
| const [state2, setState2] = useState<string>("test string") |
There was a problem hiding this comment.
Remove unnecessary TypeScript type annotations.
The explicit <string> type parameters are unnecessary since TypeScript can infer the type from the initial value. This violates the replace-unnecessary-type-definition-use-state rule.
- const [state, setState] = useState<string>("test string")
+ const [state, setState] = useState("test string")
- const [state2, setState2] = useState<string>("test string")
+ const [state2, setState2] = useState("test string")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [state, setState] = useState<string>("test string") | |
| const [state2, setState2] = useState<string>("test string") | |
| const [state, setState] = useState("test string") | |
| const [state2, setState2] = useState("test string") |
🤖 Prompt for AI Agents
In jsx_ast_grep_failure.ts around lines 7 to 9, remove the explicit <string>
type annotations from the useState calls because TypeScript can infer the type
from the initial string value. Simply call useState with the initial string
without specifying the generic type parameter to comply with the
replace-unnecessary-type-definition-use-state rule.
| const [first, second] = await Promise.all([ | ||
| await (new Promise(() => {console.log("logging this long task")})), | ||
| new Promise(() => {console.log("logging another long task")}), | ||
| ]) |
There was a problem hiding this comment.
Fix Promise.all usage and console.log violations.
Multiple issues with this code:
console.logusage violates theno-console-except-errorrule- Double
awaiton the first promise is incorrect - Promises don't resolve/reject, causing them to hang indefinitely
- const [first, second] = await Promise.all([
- await (new Promise(() => {console.log("logging this long task")})),
- new Promise(() => {console.log("logging another long task")}),
- ])
+ const [first, second] = await Promise.all([
+ new Promise((resolve) => {
+ // Removed console.log to comply with no-console-except-error rule
+ resolve("first task completed");
+ }),
+ new Promise((resolve) => {
+ // Removed console.log to comply with no-console-except-error rule
+ resolve("second task completed");
+ }),
+ ])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [first, second] = await Promise.all([ | |
| await (new Promise(() => {console.log("logging this long task")})), | |
| new Promise(() => {console.log("logging another long task")}), | |
| ]) | |
| const [first, second] = await Promise.all([ | |
| new Promise((resolve) => { | |
| // Removed console.log to comply with no-console-except-error rule | |
| resolve("first task completed"); | |
| }), | |
| new Promise((resolve) => { | |
| // Removed console.log to comply with no-console-except-error rule | |
| resolve("second task completed"); | |
| }), | |
| ]) |
🧰 Tools
🪛 ast-grep (0.38.6)
[hint] 11-11:
Context: await (new Promise(() => {console.log("logging this long task")}))
Suggested replacement: (new Promise(() => {console.log("logging this long task")}))
(no-await-in-promise-all-custom)
🤖 Prompt for AI Agents
In jsx_ast_grep_failure.ts around lines 11 to 14, fix the Promise.all usage by
removing the redundant double await on the first promise and ensure both
promises properly resolve or reject to avoid hanging. Replace console.log calls
with an allowed logging method or remove them to comply with the
no-console-except-error rule.
| export function findInString(text, string, logFn = console.error) { | ||
| logFn("text", text) | ||
| return text.includes(string) | ||
| } | ||
| return text.includes(string) | ||
| } |
There was a problem hiding this comment.
Fix duplicate code causing parse errors.
There are duplicate lines 75-76 that repeat the return statement and closing brace, causing parse errors as flagged by static analysis.
Remove the duplicate lines:
export function findInString(text, string, logFn = console.error) {
logFn("text", text)
return text.includes(string)
}
- return text.includes(string)
-}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function findInString(text, string, logFn = console.error) { | |
| logFn("text", text) | |
| return text.includes(string) | |
| } | |
| return text.includes(string) | |
| } | |
| export function findInString(text, string, logFn = console.error) { | |
| logFn("text", text) | |
| return text.includes(string) | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 75-75: Illegal return statement outside of a function
(parse)
[error] 75-76: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🤖 Prompt for AI Agents
In greet.js around lines 71 to 76, there is a duplicate return statement and
closing brace causing parse errors. Remove the repeated lines at 75 and 76 so
that the function has only one return statement and one closing brace to fix the
syntax error.
| export function welcome(name, surname) { | ||
| console.log(`Welcome, ${name} ${surname}!`) | ||
| } |
There was a problem hiding this comment.
Console.log usage violates linting rules.
The console.log statement violates the AST-grep linting rule that disallows console.log, console.debug, and console.warn except for console.error inside catch clauses.
Consider replacing with console.error if this is for error logging, or remove the console statement if it's for debugging purposes:
- console.log(`Welcome, ${name} ${surname}!`)
+ console.error(`Welcome, ${name} ${surname}!`)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function welcome(name, surname) { | |
| console.log(`Welcome, ${name} ${surname}!`) | |
| } | |
| export function welcome(name, surname) { | |
| console.error(`Welcome, ${name} ${surname}!`) | |
| } |
🤖 Prompt for AI Agents
In greet.js around lines 15 to 17, the use of console.log violates linting rules
that disallow console.log outside of catch clauses. To fix this, either remove
the console.log statement if it is for debugging or replace it with
console.error only if it is intended for error logging inside a catch block.
Adjust the code accordingly to comply with the linting restrictions.
| if (!greeting) { | ||
| console.log("No greeting") | ||
| return null | ||
| } |
There was a problem hiding this comment.
Console.log usage violates linting rules.
The console.log statement violates the AST-grep linting rule that disallows console.log, console.debug, and console.warn except for console.error inside catch clauses.
Consider replacing with console.error:
- console.log("No greeting")
+ console.error("No greeting")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!greeting) { | |
| console.log("No greeting") | |
| return null | |
| } | |
| if (!greeting) { | |
| - console.log("No greeting") | |
| + console.error("No greeting") | |
| return null | |
| } |
🤖 Prompt for AI Agents
In greet.js around lines 28 to 31, the use of console.log violates linting rules
that only allow console.error inside catch clauses. Replace the console.log("No
greeting") statement with console.error("No greeting") to comply with the
linting rules.
| export async function findInStringAsync(text, string) { | ||
| try { | ||
| return text.includes(string) | ||
| } catch (error) { | ||
| console.log("error", error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Console.log in catch block violates linting rules.
The console.log statement in the catch block violates the AST-grep linting rule. Only console.error is allowed in catch blocks.
Replace with console.error:
} catch (error) {
- console.log("error", error)
+ console.error("error", error)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function findInStringAsync(text, string) { | |
| try { | |
| return text.includes(string) | |
| } catch (error) { | |
| console.log("error", error) | |
| } | |
| } | |
| export async function findInStringAsync(text, string) { | |
| try { | |
| return text.includes(string) | |
| } catch (error) { | |
| console.error("error", error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In greet.js around lines 84 to 90, replace the console.log statement in the
catch block with console.error to comply with linting rules that only allow
console.error in catch blocks. This means changing console.log("error", error)
to console.error("error", error).
| // TODO: Don't use random in render | ||
| const num = Math.floor(Math.random() * 1e7) | ||
| .toString() | ||
| .replace(/.d+/gi, "") |
There was a problem hiding this comment.
Fix regex pattern and address TODO comment.
Two issues need attention:
- The regex pattern
/.d+/giis incorrect - it should be/\d+/gito match digits - The TODO comment highlights a valid concern about using
Math.random()in render, which can cause unnecessary re-renders
Apply this diff to fix the regex:
- .replace(/.d+/gi, "")
+ .replace(/\d+/gi, "")Would you like me to suggest an alternative approach to generate the visitor number that doesn't use random in render?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: Don't use random in render | |
| const num = Math.floor(Math.random() * 1e7) | |
| .toString() | |
| .replace(/.d+/gi, "") | |
| // TODO: Don't use random in render | |
| const num = Math.floor(Math.random() * 1e7) | |
| .toString() | |
| .replace(/\d+/gi, "") |
🤖 Prompt for AI Agents
In greet.js around lines 33 to 36, fix the regex pattern by replacing `/.d+/gi`
with `/\d+/gi` to correctly match digits. Additionally, address the TODO by
removing the use of Math.random() inside the render function to prevent
unnecessary re-renders; instead, generate the visitor number outside the render
or use a stable unique identifier approach.
Summary by CodeRabbit
New Features
Chores