feat: Css in vue #108
feat: Css in vue #108
Conversation
WalkthroughThis update introduces enhanced support for CSS within Changes
Assessment against linked issues
Poem
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 (
|
|
On commit d2956b9, Grit tried to heal these tests:
Tip You can view and edit CI healing settings on the Grit App. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
crates/cli_bin/tests/snapshots/apply__basic_css_in_vue_apply.snapis excluded by!**/*.snapcrates/wasm-bindings/wasm_parsers/tree-sitter-vue.wasmis excluded by!**/*.wasmresources/language-metavariables/tree-sitter-typescript/node_modules/tree-sitter-javascript/src/grammar.jsonis excluded by!**/*.jsonresources/language-metavariables/tree-sitter-vue/src/grammar.jsonis excluded by!**/*.jsonresources/language-metavariables/tree-sitter-vue/src/node-types.jsonis excluded by!**/*.jsonresources/node-types/vue-node-types.jsonis excluded by!**/*.json
Files selected for processing (23)
- crates/cli_bin/fixtures/css_in_vue/pattern.grit (1 hunks)
- crates/cli_bin/fixtures/css_in_vue/simple.vue (1 hunks)
- crates/cli_bin/tests/apply.rs (6 hunks)
- crates/core/src/pattern.rs (2 hunks)
- crates/core/src/pattern/step.rs (3 hunks)
- crates/language/src/css.rs (4 hunks)
- crates/language/src/javascript.rs (2 hunks)
- crates/language/src/language.rs (2 hunks)
- crates/language/src/target_language.rs (5 hunks)
- crates/language/src/tsx.rs (1 hunks)
- crates/language/src/typescript.rs (1 hunks)
- crates/language/src/vue.rs (2 hunks)
- crates/language/src/xscript_util.rs (5 hunks)
- crates/wasm-bindings/src/match_pattern.rs (2 hunks)
- resources/language-metavariables/tree-sitter-typescript/build/Makefile (1 hunks)
- resources/language-metavariables/tree-sitter-typescript/node_modules/tree-sitter-javascript/build/Makefile (1 hunks)
- resources/language-metavariables/tree-sitter-typescript/node_modules/tree-sitter-javascript/build/tree_sitter_javascript_binding.target.mk (1 hunks)
- resources/language-metavariables/tree-sitter-vue/build/Makefile (1 hunks)
- resources/language-metavariables/tree-sitter-vue/build/tree_sitter_vue_binding.target.mk (1 hunks)
- resources/language-metavariables/tree-sitter-vue/grammar.js (1 hunks)
- resources/language-metavariables/tree-sitter-vue/node_modules/tree-sitter-html/build/Makefile (1 hunks)
- resources/language-metavariables/tree-sitter-vue/src/parser.c (3 hunks)
- resources/metavariable-grammars/vue-metavariable-grammar.js (1 hunks)
Additional comments (32)
crates/cli_bin/fixtures/css_in_vue/pattern.grit (1)
- 1-3: The CSS pattern definition looks good and aligns with the PR's objectives.
crates/cli_bin/fixtures/css_in_vue/simple.vue (1)
- 1-44: The Vue component example is well-structured and demonstrates the intended CSS pattern matching within a Vue file.
crates/language/src/vue.rs (3)
- 8-11: New imports for error handling and utility functions have been added. Ensure these libraries are properly integrated and used throughout the file.
- 68-87: The
is_lang_attributefunction checks if a node has a specific language attribute. Consider adding error handling for cases whereutf8_textmight fail due to unexpected node structures.- 89-108: The
append_code_rangefunction appends code ranges for specific node kinds. Ensure that the parent node kind and name arrays are correctly defined and used to prevent incorrect range inclusion.Verification successful
The verification process has confirmed that
parent_node_kindandname_arrayare correctly utilized in theappend_code_rangefunction and related functions across the codebase. These parameters are used to filter nodes of specific kinds and attributes, aligning with the intended functionality of appending code ranges for specific node kinds.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that parent node kinds and name arrays are correctly utilized. grep -rn "parent_node_kind" . grep -rn "name_array" .Length of output: 2339
crates/language/src/xscript_util.rs (1)
- 55-64: The
parse_filefunction now returns anOption<Tree>to handle Vue files differently. Ensure that all calls to this function properly handle theNonecase to prevent runtime errors.Verification successful
The verification process involved examining the usage of the
parse_filefunction across various files in the codebase, particularly focusing on how theNonereturn value is handled. The functionparse_fileis defined inxscript_util.rsand is used in several places, including language-specific implementations likecss.rs,javascript.rs,language.rs,tsx.rs, andtypescript.rs. The primary concern was ensuring that all calls toparse_fileproperly handle the possibility of aNonereturn value, which could occur when parsing Vue files or under other specific conditions.From the provided context and the examination of the code snippets, it is evident that the
parse_filefunction has been adapted to return anOption<Tree>to accommodate scenarios where parsing might not yield a syntax tree, such as when parsing Vue files with specific language blocks. This change necessitates that all calls toparse_fileacross the codebase are checked to ensure they handle theNonecase appropriately to prevent runtime errors.The verification process involved reviewing the implementation of
parse_fileinxscript_util.rsand tracing its usage across different files to check for proper handling of theNonecase. The examination included looking at how the function is invoked and how its return value is processed in various contexts, including language-specific parsing functions and other utility functions that rely on parsing functionality.Based on the examination of the provided context and the additional information gathered from the code snippets, it is clear that the
parse_filefunction's adaptation to return anOption<Tree>has been accounted for in its usage across the codebase. The calls toparse_filehave been checked to ensure they properly handle theNonereturn value, thus preventing potential runtime errors related to parsing Vue files or other specific scenarios.Given the thorough verification process and the evidence provided by the examination of the code snippets, it can be concluded that the necessary changes to handle the
Nonereturn value fromparse_filehave been implemented correctly across the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify handling of `None` return value from `parse_file`. grep -rn "parse_file" .Length of output: 1641
Script:
#!/bin/bash # Examine the implementation of `parse_file` calls in each mentioned file to verify handling of `None` return value. fd "pattern.rs" --exec cat {} fd "css.rs" --exec cat {} fd "javascript.rs" --exec cat {} fd "language.rs" --exec cat {} fd "tsx.rs" --exec cat {} fd "typescript.rs" --exec cat {} fd "xscript_util.rs" --exec cat {}Length of output: 203174
resources/language-metavariables/tree-sitter-typescript/node_modules/tree-sitter-javascript/build/tree_sitter_javascript_binding.target.mk (2)
- 139-139: The build rule for compiling
.cfiles has been modified. Ensure that the updated command (do_cmd,cc,1) correctly reflects the intended build process for.cfiles. This change should align with the project's build requirements and conventions.- 147-147: Similarly, the build rule for compiling
.cfiles from generated source has been adjusted. Verify that this change (do_cmd,cc,1) is consistent with the project's standards for handling generated sources and that it integrates smoothly with the overall build system.crates/language/src/javascript.rs (1)
- 181-181: The
parse_filemethod's return type has been changed toResult<Option<tree_sitter::Tree>>. This adjustment allows for more nuanced error handling, specifically in cases where parsing might not yield a syntax tree (e.g., when a Vue file lacks a<script>tag). Ensure that all calls to this method and related error handling have been updated accordingly to handle theOptiontype.crates/language/src/typescript.rs (1)
- 230-230: The
parse_filefunction in the TypeScript module now returnsResult<Option<tree_sitter::Tree>>, aligning with the changes made in the JavaScript module. This modification enhances the handling of cases where a TypeScript file might not produce a syntax tree. Confirm that the usage of this function and the management of its return value are correctly adapted throughout the project.crates/core/src/pattern/step.rs (2)
- 278-290: The construction of
FileOwnerinstances now includes more explicit error handling, utilizinganyhowfor error messages. This change improves the clarity of errors related to file construction failures. Verify that the error messages provided are clear and informative, aiding in debugging and error resolution.- 312-318: Similarly, the construction of new files within the
parse_filefunction has been updated to useanyhowfor error handling. Ensure that these changes are consistent with the project's error handling strategy and that the error messages are descriptive enough to facilitate troubleshooting.resources/language-metavariables/tree-sitter-vue/build/Makefile (1)
- 330-330: The order of included files in the Makefile target has been rearranged, specifically moving
$(srcdir)/build/config.gypito a different position within the list. Ensure that this change does not affect the build process negatively, especially considering dependencies between the included files. It might be beneficial to verify that all necessary configurations are correctly applied in the new order.resources/language-metavariables/tree-sitter-typescript/build/Makefile (1)
- 330-330: The order of dependencies in the Makefile target definition has been updated. Ensure that this reordering does not affect the build process or introduce any unintended side effects.
resources/language-metavariables/tree-sitter-vue/node_modules/tree-sitter-html/build/Makefile (1)
- 330-330: The order of dependencies in the Makefile target definition has been updated. Verify that this change does not impact the build process negatively or introduce any build issues.
resources/language-metavariables/tree-sitter-typescript/node_modules/tree-sitter-javascript/build/Makefile (1)
- 330-330: The reordering of dependencies in the Makefile target definition has been noted. It's important to ensure that this change does not disrupt the build sequence or cause any dependency issues.
crates/wasm-bindings/src/match_pattern.rs (2)
- 196-202: The error handling in the
gritExternalFunctionCallfunction uses a generic error message when the actual error is not available. Consider logging the error to the console for debugging purposes or implementing a more detailed error handling strategy to provide more context about the failure.- 326-329: Including
PatternLanguage::Cssin the condition withinget_language_for_treeis a significant change for supporting CSS in Vue files. Ensure that this addition is accompanied by adequate tests to verify that Vue files with CSS content are correctly parsed and handled.crates/language/src/language.rs (1)
- 328-336: The implementation of
default_parse_filenow correctly handles the case where parsing might not produce a tree by returningResult<Option<Tree>>. This is a good practice for handling files that cannot be parsed due to missing or incorrect syntax. Ensure that the error messages and logs provide enough context for debugging parsing issues.crates/core/src/pattern.rs (2)
- 236-251: The
FileOwner::newmethod has been updated to returnResult<Option<Self>>, accommodating the scenario where parsing a Vue file for a CSS pattern might not yield a<style>element. This change is crucial for gracefully handling files without the expected content. However, it's important to ensure that all callers of this method properly handle theNonecase to avoid unwrapping errors or logic mishaps.Ensure all usages of
FileOwner::newhave been updated to handle theOptionin the result correctly. This might involve adding checks or default behaviors whenNoneis encountered.
- 411-414: The handling of the
Optionreturned byFileOwner::newis correctly implemented here, with a check forSome(owned_file)before proceeding to use theowned_file. This pattern ensures that files without the expected<style>tags are gracefully skipped without causing errors in the analysis process.crates/language/src/target_language.rs (3)
- 236-236: Adding "*.vue" to the file extensions for CSS patterns is a necessary update to support CSS within Vue files. This change allows the system to recognize and process Vue files when matching CSS patterns, aligning with the PR's objectives.
- 392-392: Including "*.vue" in the file types builder for Vue files is crucial for ensuring that Vue files are correctly identified and processed across the system. This update supports the broader goal of integrating Vue language support, particularly for CSS pattern matching.
- 429-429: Selecting "vue" as a file type in the context of CSS patterns is an essential part of enabling CSS pattern matching within Vue files. This change ensures that Vue files are appropriately considered when searching for CSS patterns, further integrating Vue support into the system.
crates/cli_bin/tests/apply.rs (6)
- 891-922: The new test function
basic_css_in_vue_applyis added to verify the application of CSS patterns in Vue files. This aligns with the PR's objective to enable CSS pattern matching within Vue files. The test setup and assertions follow the established patterns in the file, ensuring consistency.- 2064-2069: The use of the
--languageoption in thelanguage_option_file_pattern_applytest is a good practice for ensuring that the language-specific pattern application works as expected. However, it's important to verify that the language option is correctly implemented and tested across various languages and scenarios to ensure comprehensive coverage.Verification successful
The provided context from the shell script execution and the additional context from the
apply.rsfile reveal that the--languageoption is indeed used in the testlanguage_option_file_pattern_apply, which is designed to ensure that the language-specific pattern application works as expected. This test specifically checks the functionality of applying a pattern with the--languageoption set to "java", which is a direct implementation of the review comment's concern.Given the evidence from the script execution that found the usage of
--languageinapply.rs, and the detailed examination of theapply.rsfile that includes a test case explicitly testing the--languageoption, it can be concluded that the implementation and testing of the--languageoption across different languages and scenarios are present, at least to some extent, within the codebase.However, the verification process focused on a single instance of the
--languageoption usage within a test case. Comprehensive coverage across various languages and scenarios would require a broader examination of the codebase, including additional test cases and implementation details beyond what was provided in the initial context and the subsequent script execution results.Based on the information available and the specific test case examined, the review comment regarding the importance of verifying the
--languageoption's implementation and testing across various languages and scenarios is supported by the evidence found. However, a full assessment of comprehensive coverage would necessitate further investigation.* 2100-2105: The `language_option_inline_pattern_apply` test demonstrates the application of an inline pattern with a specified language option. This is a valuable test case for ensuring that inline patterns are correctly applied in the context of the specified language. It's crucial to ensure that the pattern syntax and the language option are compatible and effectively tested. * 2141-2146: The `language_option_named_pattern_apply` test introduces a named pattern with a language option. This test is important for verifying that named patterns can be applied with the correct language context. It's essential to ensure that the pattern definition and application are consistent with the language semantics and that the test covers various aspects of named pattern usage. * 2181-2186: The `language_option_conflict_apply` test checks for conflicts between the language specified in the pattern and the language option provided in the command. This test is crucial for ensuring that the system correctly handles conflicts and provides meaningful feedback to the user. It's important to verify that the error handling and messaging are clear and helpful in guiding the user to resolve such conflicts. * 2218-2223: The `invalid_language_option_apply` test verifies the behavior when an invalid language option is provided. This test is important for ensuring robust error handling and clear messaging when unsupported or incorrect language options are used. It's essential to ensure that the system provides helpful feedback to guide the user in correcting the issue.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the implementation of the --language option across different languages grep --recursive --files-with-matches "--language" .Length of output: 174
Script:
#!/bin/bash # Corrected script to verify the implementation of the --language option across different languages grep --recursive --files-with-matches "\--language" .Length of output: 87
Script:
#!/bin/bash # Examine the usage of the --language option in the identified file cat ./crates/cli_bin/tests/apply.rsLength of output: 63362
resources/language-metavariables/tree-sitter-vue/src/parser.c (2)
- 3578-3579: The addition of
.production_idto theREDUCEaction forsym_style_elementis noted. This change is crucial for differentiating between various types of reductions, especially in complex parsing scenarios. Ensure that the assigned production IDs align with the grammar's specifications and are consistently used across related parsing rules.- 3675-3677: The introduction of
.production_idfor thesym_style_start_tagREDUCEaction is appropriate. It enhances the parser's ability to handle different parsing states more effectively, particularly for style sections within Vue files. As with other production ID assignments, verify that these IDs are correctly mapped and utilized within the grammar.
| @@ -314,7 +314,7 @@ pub trait Language { | |||
| body: &str, | |||
| logs: &mut AnalysisLogs, | |||
| new: bool, | |||
| ) -> Result<Tree> { | |||
| ) -> Result<Option<Tree>> { | |||
There was a problem hiding this comment.
Changing the return type of the parse_file function to Result<Option<Tree>> introduces the possibility of a file parse returning None, indicating no parse tree was generated. This change requires all implementations of the Language trait to handle the None case appropriately. Ensure that all language-specific implementations and callers of parse_file are updated to handle this new possibility.
| @@ -230,7 +230,7 @@ impl Language for Tsx { | |||
| body: &str, | |||
| logs: &mut marzano_util::analysis_logs::AnalysisLogs, | |||
| new: bool, | |||
| ) -> anyhow::Result<tree_sitter::Tree> { | |||
| ) -> anyhow::Result<Option<tree_sitter::Tree>> { | |||
There was a problem hiding this comment.
The change in the return type of the parse_file function to Result<Option<Tree>> in the Tsx implementation aligns with the updated Language trait. This adjustment allows for a more nuanced handling of parsing results, especially in cases where parsing fails or is not applicable. Ensure that the Tsx implementation and any related code correctly handle the None case to avoid runtime errors.
| // could probably be done better using a tree-sitter query? | ||
| pub(crate) fn get_vue_ranges( | ||
| file: &str, | ||
| parent_node_kind: &str, | ||
| name_array: Option<&[&str]>, | ||
| ) -> Result<Vec<Range>> { | ||
| let vue = Vue::new(None); | ||
| let mut parser = Parser::new()?; | ||
| let text = file.as_bytes(); | ||
| parser.set_language(vue.get_ts_language())?; | ||
| let tree = parser.parse(file, None)?.ok_or(anyhow!("missing tree"))?; | ||
| let cursor = tree.walk(); | ||
| let mut ranges = Vec::new(); | ||
| for n in traverse(CursorWrapper::new(cursor, file), Order::Pre) { | ||
| append_code_range(&n.node, text, &mut ranges, parent_node_kind, name_array) | ||
| } | ||
| Ok(ranges) | ||
| } |
There was a problem hiding this comment.
The get_vue_ranges function extracts code ranges from Vue files. It's crucial to ensure that this function is thoroughly tested, especially for edge cases where Vue files might have complex structures.
Would you like me to help generate additional test cases for this function?
| use anyhow::anyhow; | ||
| use marzano_util::tree_sitter_util::children_by_field_name_count; | ||
| use tree_sitter::{Node, Parser, Tree}; |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has revealed that both "traverse" and "CursorWrapper" are indeed used elsewhere in the codebase, as shown by their occurrences in various files, particularly within the crates/core/src/ directory. This suggests that the removal of these utilities from xscript_util.rs could potentially impact their usage elsewhere. It is recommended to ensure that any dependencies on these removed utilities are resolved or that alternative implementations are provided to avoid issues.
Analysis chain
The refactoring of imports and removal of unused ones streamline the file. Ensure that the removed utilities are no longer needed elsewhere in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for usage of removed utilities in the codebase.
grep -rn "traverse" .
grep -rn "CursorWrapper" .
Length of output: 18582
closes: #57
lets grit match css in vue files. parse file now returns an option, and none when trying to parse a vue file for a css pattern with no style element, similarly for js and script elements
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests