feat: support more languages in napi#1171
Conversation
|
Warning Rate Limit Exceeded@HerringtonDarkholme has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 38 minutes and 32 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe changes introduce support for various new programming languages in the NAPI module. This includes updating enums to string literals, adding parsing and processing functions, and modifying structs and methods to handle these new languages. Additionally, tests and file handling logic have been adjusted to accommodate the expanded language support. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant NAPI
participant SupportLang
User->>NAPI: Call parse(lang, src)
NAPI->>SupportLang: Convert lang to SupportLang
SupportLang->>NAPI: Return converted lang
NAPI->>User: Return parsed SgRoot
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1171 +/- ##
==========================================
+ Coverage 73.43% 73.57% +0.14%
==========================================
Files 76 76
Lines 5239 5229 -10
==========================================
Hits 3847 3847
+ Misses 1392 1382 -10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
crates/napi/index.d.ts (1)
Line range hint
12-12: Consider specifying more precise types instead ofanyfor better type safety.- rule: any + rule: RuleType // Define RuleType based on expected structureAlso applies to: 14-14, 18-18, 20-20
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!**/*.lock
Files selected for processing (6)
- crates/napi/Cargo.toml (1 hunks)
- crates/napi/index.d.ts (1 hunks)
- crates/napi/src/doc.rs (7 hunks)
- crates/napi/src/fe_lang.rs (5 hunks)
- crates/napi/src/lib.rs (6 hunks)
- crates/napi/src/sg_node.rs (2 hunks)
Additional Context Used
Biome (5)
crates/napi/index.d.ts (5)
12-12: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
18-18: Unexpected any. Specify a different type.
20-20: Unexpected any. Specify a different type.
22-46: The enum declaration should not be const
Additional comments not posted (5)
crates/napi/Cargo.toml (1)
16-16: Dependency addition looks good and aligns with the PR's objectives.crates/napi/src/doc.rs (1)
6-6: Changes to useSupportLangare correctly implemented and align with the PR's objectives to support more languages.Also applies to: 32-34, 126-126, 131-131, 140-140, 172-172, 181-181, 193-193
crates/napi/src/fe_lang.rs (1)
1-1: Expansion ofFrontEndLanguageand implementation of conversion methods are correctly done to support additional languages.Also applies to: 13-38, 41-69, 73-100, 105-105, 113-120, 129-131, 146-149, 158-167, 180-185
crates/napi/src/sg_node.rs (1)
183-183: Changes inSgNodeto use the new language support are correctly implemented.Also applies to: 223-223
crates/napi/src/lib.rs (1)
11-11: Integration ofSupportLangand updates to file handling are correctly implemented.Also applies to: 36-36, 54-55, 103-103, 229-229
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (2)
crates/napi/src/find_files.rs (2)
19-22: Define the purpose ofParseAsyncstruct clearly in a comment.Adding a comment explaining the role of
ParseAsyncwould improve code readability and maintainability.
40-45: Clarify the type parameterDinIterateFiles.It would be beneficial to add a comment explaining the purpose and expected type of the generic parameter
Din theIterateFilesstruct.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- crates/napi/src/find_files.rs (1 hunks)
- crates/napi/src/lib.rs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/napi/src/lib.rs
Additional comments not posted (4)
crates/napi/src/find_files.rs (4)
101-122: Review the error handling inparse_filesfunction.Verification successful
The
parse_filesfunction employs standard Rust error handling practices, including the use of theResulttype and the?operator to propagate errors. Here are the key points of error handling within the function:
- The creation of the
ThreadsafeFunctionuses the?operator to handle potential errors.- The
matchstatement correctly handles different variants ofpaths.- The call to
build_filesalso uses the?operator to propagate errors.- The final
Okresult is correctly constructed with the necessary components.Based on this detailed inspection, the error handling in the
parse_filesfunction appears to be robust and follows best practices.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that error handling is robust in `parse_files`. # Test: Search for error handling patterns in `parse_files`. ast-grep --lang rust --pattern $'fn parse_files($_, $_) { $$$ }'Length of output: 65
Script:
#!/bin/bash # Locate the `parse_files` function definition in the codebase. rg 'fn parse_files' -t rust # Once located, extract the function definition and its body for detailed inspection. # Assuming the function is found in `crates/napi/src/find_files.rs`, extract lines around its definition. rg -A 20 -B 5 'fn parse_files' crates/napi/src/find_files.rsLength of output: 1008
178-199: Validate the logic infind_in_files_impl.Verification successful
The logic in
find_in_files_implappears to be correctly implemented based on the components used:
create_threadsafe_functionis used to create a threadsafe function from the callback.parse_withis used to parse a rule with the matcher.find_filesis used to find files using the language-specific method.AsyncTask::newis used to create a new asynchronous task with the gathered data.These components are used appropriately within the function, and their definitions and usages align with the intended functionality.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic in `find_in_files_impl` matches the intended functionality. # Test: Search for the implementation of `find_in_files_impl`. ast-grep --lang rust --pattern $'fn find_in_files_impl($_, $_, $_) { $$$ }'Length of output: 76
Script:
#!/bin/bash # Description: Verify the logic in `find_in_files_impl` matches the intended functionality. # Test: Search for the implementation of `find_in_files_impl` in the entire codebase. rg 'fn find_in_files_impl' -A 20Length of output: 1249
Script:
#!/bin/bash # Description: Verify the components used in `find_in_files_impl`. # Test: Search for the definition and usage of `create_threadsafe_function`. rg 'create_threadsafe_function' -A 5 # Test: Search for the definition and usage of `parse_with`. rg 'parse_with' -A 5 # Test: Search for the definition and usage of `find_files`. rg 'find_files' -A 5 # Test: Search for the definition and usage of `AsyncTask::new`. rg 'AsyncTask::new' -A 5Length of output: 7380
124-142: Ensure thatcall_sg_rootfunction handles all possible errors.
28-32: Ensure proper error handling incomputemethod ofParseAsync.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (2)
crates/napi/index.js (1)
Line range hint
7-8: Use thenode:protocol for built-in modules.- const { existsSync, readFileSync } = require('fs') + const { existsSync, readFileSync } = require('node:fs') - const { join } = require('path') + const { join } = require('node:path') - const lddPath = require('child_process').execSync('which ldd').toString().trim() + const lddPath = require('node:child_process').execSync('which ldd').toString().trim()Also applies to: 20-20
crates/napi/index.d.ts (1)
Line range hint
12-12: Consider replacinganywith more specific types to improve type safety and code clarity.// Example for `rule` in `NapiConfig` rule: RuleConfig // Assuming RuleConfig is an appropriate typeAlso applies to: 14-14, 18-18, 20-20
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- crates/napi/index.d.ts (2 hunks)
- crates/napi/index.js (1 hunks)
- crates/napi/src/lib.rs (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/napi/src/lib.rs
Additional Context Used
Biome (9)
crates/napi/index.d.ts (5)
12-12: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
18-18: Unexpected any. Specify a different type.
20-20: Unexpected any. Specify a different type.
22-46: The enum declaration should not be const
crates/napi/index.js (4)
7-7: A Node.js builtin module should be imported with the node: protocol.
8-8: A Node.js builtin module should be imported with the node: protocol.
20-20: A Node.js builtin module should be imported with the node: protocol.
281-281: Do not use template literals if interpolation and special-character handling are not needed.
Additional comments not posted (2)
crates/napi/index.js (1)
284-294: The module exports are correctly structured and follow Node.js best practices.crates/napi/index.d.ts (1)
23-45: The update to string literals in theFrontEndLanguageenum enhances readability. However, remember that this file is auto-generated, and manual changes might be overwritten.
napi is no longer frontend only
BREAKING CHANGE: rename FrontendLanguage to Lang
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (3)
crates/napi/__test__/index.spec.ts (1)
3-3: The imports should be organized and possibly consolidated to improve readability and maintainability.Consider organizing imports in a grouped manner:
import { js, parseFiles, ts, tsx, html, parse as parseWithLang } from '../index';crates/napi/index.js (1)
Line range hint
7-7: Consider using thenode:protocol when importing Node.js built-in modules to ensure compatibility with newer Node.js versions.- const { existsSync, readFileSync } = require('fs') + const { existsSync, readFileSync } = require('node:fs') - const { join } = require('path') + const { join } = require('node:path') - const lddPath = require('child_process').execSync('which ldd').toString().trim() + const lddPath = require('node:child_process').execSync('which ldd').toString().trim()Also applies to: 8-8, 20-20
crates/napi/index.d.ts (1)
Line range hint
12-12: Consider replacing theanytype with more specific types to improve type safety and maintainability.interface NapiConfig { rule: RuleType; // Define RuleType based on the structure expected constraints?: ConstraintsType; // Define ConstraintsType based on the structure expected transform?: TransformType; // Define TransformType based on the structure expected utils?: UtilsType; // Define UtilsType based on the structure expected }Also applies to: 14-14, 18-18, 20-20
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- crates/napi/test/index.spec.ts (2 hunks)
- crates/napi/index.d.ts (2 hunks)
- crates/napi/index.js (1 hunks)
- crates/napi/src/doc.rs (7 hunks)
- crates/napi/src/find_files.rs (1 hunks)
- crates/napi/src/lib.rs (3 hunks)
- crates/napi/src/napi_lang.rs (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- crates/napi/src/doc.rs
Additional Context Used
Biome (29)
crates/napi/__test__/index.spec.ts (20)
11-11: Forbidden non-null assertion.
15-15: Forbidden non-null assertion.
16-16: Forbidden non-null assertion.
25-25: Forbidden non-null assertion.
29-29: Forbidden non-null assertion.
30-30: Forbidden non-null assertion.
39-39: Forbidden non-null assertion.
43-43: Forbidden non-null assertion.
55-56: Forbidden non-null assertion.
60-61: Forbidden non-null assertion.
70-83: Forbidden non-null assertion.
90-91: Forbidden non-null assertion.
134-137: Forbidden non-null assertion.
134-134: Forbidden non-null assertion.
141-141: Forbidden non-null assertion.
152-152: Forbidden non-null assertion.
173-174: Unnecessary continue statement
185-185: The computed expression can be simplified without the use of a string literal.
220-220: Unnecessary continue statement
284-284: Unexpected any. Specify a different type.
crates/napi/index.d.ts (5)
12-12: Unexpected any. Specify a different type.
14-14: Unexpected any. Specify a different type.
18-18: Unexpected any. Specify a different type.
20-20: Unexpected any. Specify a different type.
39-63: The enum declaration should not be const
crates/napi/index.js (4)
7-7: A Node.js builtin module should be imported with the node: protocol.
8-8: A Node.js builtin module should be imported with the node: protocol.
20-20: A Node.js builtin module should be imported with the node: protocol.
281-281: Do not use template literals if interpolation and special-character handling are not needed.
Additional comments not posted (9)
crates/napi/src/lib.rs (5)
4-5: Ensure all newly imported modules are utilized effectively in the file.Also applies to: 15-17
Verification successful
The newly imported modules
find_filesandnapi_langare indeed utilized within the filecrates/napi/src/lib.rs. The references include:
use find_files::{find_in_files_impl, FindConfig, FindInFiles, ParseAsync};use napi_lang::Lang;pub use find_files::parse_files;These usages confirm that the modules are effectively integrated into the code.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the newly imported modules are used in the file. # Test: Search for usage of the modules. Expect: References to these modules in the code. rg --type rust --files-with-matches 'find_files|napi_lang' crates/napi/src/lib.rsLength of output: 106
Script:
#!/bin/bash # Description: Verify specific usages of the modules `find_files` and `napi_lang` in the file. # Test: Search for function calls or usage patterns related to these modules. rg --type rust 'find_files|napi_lang' crates/napi/src/lib.rsLength of output: 222
68-72: The use of aliases for functions to resolve name conflicts is a good practice. However, ensure that these aliases do not introduce any unexpected behaviors or bugs.Verification successful
The aliases
kind_with_lang,parse_with_lang,parse_async_with_lang, andpattern_with_langare used consistently within thecrates/napi/src/lib.rsfile. The context around their usage indicates that they are correctly mapped to their respective functions without introducing unexpected behaviors.
- Lines 31, 41, 46, and 51 show the aliases being used in function definitions.
- The surrounding code confirms that these aliases are used appropriately and maintain the expected behavior of the original functions.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the function aliases do not introduce unexpected behaviors. # Test: Search for usages of the aliases. Expect: Consistent behavior with the original functions. rg --type rust --files-with-matches 'kind_with_lang|parse_with_lang|parse_async_with_lang|pattern_with_lang' crates/napi/src/lib.rsLength of output: 156
Script:
#!/bin/bash # Extract lines where the aliases are used in crates/napi/src/lib.rs rg 'kind_with_lang|parse_with_lang|parse_async_with_lang|pattern_with_lang' crates/napi/src/lib.rs -nLength of output: 430
Script:
#!/bin/bash # Extract surrounding lines for each alias usage in crates/napi/src/lib.rs rg 'kind_with_lang|parse_with_lang|parse_async_with_lang|pattern_with_lang' crates/napi/src/lib.rs -A 3 -B 3Length of output: 1260
99-103: The functionkindandpatterninvolve complex operations with potential for optimization. Review these functions for any possible improvements in efficiency or error handling.Also applies to: 108-109
82-84: The functionsparseandparse_asyncare critical for the functionality of the library. Review their implementations for any potential issues with memory management, especially with the use ofJsDoc::newandAsyncTask::new.Also applies to: 93-94
26-26: The helper functionsparse_with_lang,parse_async_with_lang,kind_with_lang, andpattern_with_langare used to abstract common functionality. Ensure these functions handle errors appropriately and are optimized for performance.Also applies to: 31-31, 41-41, 46-46, 51-51
crates/napi/src/find_files.rs (1)
19-22: TheParseAsyncstruct and its implementation of theTasktrait are well-defined. Ensure that thecomputeandresolvemethods handle all possible errors and edge cases.Also applies to: 24-36
crates/napi/src/napi_lang.rs (1)
13-39: TheLangenum and its conversions to and fromSupportLangare crucial for the functionality of the library. Ensure that all language conversions are correctly implemented and cover all cases.Also applies to: 41-71, 73-103
Verification successful
The implementations for
Into<SupportLang>andFrom<SupportLang>incrates/napi/src/napi_lang.rsappear to cover all the variants of theLangenum. Each variant ofLangis correctly mapped to its correspondingSupportLangvariant and vice versa.
Into<SupportLang>implementation (lines 41-71)From<SupportLang>implementation (lines 73-103)These implementations ensure that all language conversions are correctly handled.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correctness of language conversions. # Test: Search for the implementation of conversions. Expect: Correct and complete implementation. rg --type rust --files-with-matches 'Into<SupportLang>|From<SupportLang>' crates/napi/src/napi_lang.rsLength of output: 133
Script:
#!/bin/bash # Extract and display the implementations of `Into<SupportLang>` and `From<SupportLang>` from `crates/napi/src/napi_lang.rs` ast-grep --lang rust --pattern 'impl Into<SupportLang> for Lang { $$$ }' crates/napi/src/napi_lang.rs ast-grep --lang rust --pattern 'impl From<SupportLang> for Lang { $$$ }' crates/napi/src/napi_lang.rsLength of output: 3741
crates/napi/index.js (1)
284-294: The export statements forparseFiles,Lang, and other entities have been added. Ensure that these entities are properly defined and implemented in the native bindings.crates/napi/index.d.ts (1)
16-16: The TypeScript definitions have been updated to include new interfaces and functions, and theLangenum has been expanded to include more languages. Ensure that these changes are reflected in the implementation and that they align with the project's goals to support more languages.Also applies to: 22-26, 27-38, 39-62, 86-106
There was a problem hiding this comment.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- crates/napi/test/index.spec.ts (2 hunks)
Additional Context Used
Biome (20)
crates/napi/__test__/index.spec.ts (20)
15-15: Forbidden non-null assertion.
19-19: Forbidden non-null assertion.
20-20: Forbidden non-null assertion.
29-29: Forbidden non-null assertion.
33-33: Forbidden non-null assertion.
34-34: Forbidden non-null assertion.
43-43: Forbidden non-null assertion.
47-47: Forbidden non-null assertion.
59-60: Forbidden non-null assertion.
64-65: Forbidden non-null assertion.
74-87: Forbidden non-null assertion.
94-95: Forbidden non-null assertion.
138-141: Forbidden non-null assertion.
138-138: Forbidden non-null assertion.
145-145: Forbidden non-null assertion.
156-156: Forbidden non-null assertion.
177-178: Unnecessary continue statement
189-189: The computed expression can be simplified without the use of a string literal.
224-224: Unnecessary continue statement
288-288: Unexpected any. Specify a different type.
Additional comments not posted (2)
crates/napi/__test__/index.spec.ts (2)
308-315: The test for parsing Python code is well-implemented and correctly tests the basic functionality.
317-324: The asynchronous test for parsing Python code is correctly implemented and follows the new enhancements.
fix #1170