crates/cli/src/utils/args.rs (1)

131-131: LGTM: Attribute update improves consistency.

The change from value_name="style" to value_name="STYLE" aligns with the documentation update and ensures consistency in the command-line help output. This improvement enhances the user experience by maintaining a uniform style across documentation and usage.

For complete consistency, consider updating the num_args and require_equals attributes to use the same quotation style as value_name. For example:

-      num_args(0..=1),
-      require_equals = true,
+      num_args = "0..=1",
+      require_equals = "true",

This change would make the attribute styles uniform throughout the struct field.

crates/cli/src/scan.rs (2)

26-38: LGTM! Consider adding documentation comments.

The SeverityArg struct is well-implemented and follows good practices for defining command-line arguments using clap. The structure allows for flexible specification of rule IDs for different severity levels.

Consider adding documentation comments to the struct and its fields to provide more context for users and maintainers. For example:

/// Represents command-line arguments for specifying rule severities
#[derive(Args, Debug)]
struct SeverityArg {
    /// Rule IDs to be treated as errors
    #[clap(long, action = clap::ArgAction::Append, value_name = "RULE_ID", num_args(0..), require_equals = true)]
    pub error: Option<Vec<String>>,
    // ... (add similar comments for other fields)
}

---

76-78: LGTM! Consider using doc comments for consistency.

The integration of SeverityArg into ScanArg is well-implemented. The use of #[clap(flatten)] is appropriate for incorporating the severity options into the main command-line interface.

For consistency with Rust documentation practices, consider using a doc comment (///) instead of a regular comment for the "severity related options" line:

/// Severity related options
#[clap(flatten)]
severity: SeverityArg,

This change would make the comment style consistent with other documentation in the codebase and allow tools like rustdoc to include this information in generated documentation.

Configuration used: CodeRabbit UI
Review profile: CHILL

crates/cli/src/utils/mod.rs (1)

4-4: LGTM. Consider adding a brief comment.

The addition of the rule_overwrite module aligns with the PR's objective of adding severity override functionality.

Consider adding a brief comment above the module declaration to explain its purpose, e.g.:

// Module for handling rule severity overrides
mod rule_overwrite;

crates/cli/src/utils/args.rs (1)

231-243: LGTM: Well-structured SeverityArg for fine-grained severity control.

The new SeverityArg struct provides a clear and flexible way to override rule severities:

1. Each severity level (error, warning, info, hint, off) can accept multiple rule IDs.
2. The use of Option<Vec<String>> allows for easy handling of cases where no rules are specified for a level.
3. The require_equals = true attribute enforces clear syntax for specifying values.

Consider adding a brief doc comment to the SeverityArg struct explaining its purpose and usage. This would improve the self-documentation of the code. For example:

/// Represents command-line arguments for overriding rule severities.
/// Each field corresponds to a severity level and can accept multiple rule IDs.
#[derive(Args, Debug)]
pub struct SeverityArg {
    // ... existing fields ...
}

crates/cli/src/scan.rs (3)

62-64: LGTM: SeverityArg integration looks good

The addition of the severity field of type SeverityArg to the ScanArg struct is a good enhancement. The use of #[clap(flatten)] is appropriate for organizing related CLI arguments.

Consider adding a brief comment explaining the purpose of the severity field and how it affects the scanning process.

---

Line range hint 1-464: Summary: Severity override foundation laid, implementation incomplete

The changes introduce a new SeverityArg struct and integrate it into the ScanArg struct, laying the groundwork for implementing severity overrides. However, the implementation is not yet complete as the new severity information is not being used in the scanning process.

Next steps:

1. Update relevant functions (e.g., run_with_config, run_scan) to utilize the new severity information.
2. Add tests to verify the behavior of severity overrides.
3. Update documentation to explain how users can use the new severity override feature.

Consider creating a new function to handle the application of severity overrides to the rules. This function could be called from run_with_config or run_scan and would modify the rule severities based on the user-provided overrides.

---

Unused severity field in ScanArg

The SeverityArg is included in the ScanArg struct, but it's not utilized in the run_with_config or run_scan functions. Please ensure that the severity overrides are appropriately applied within these functions to leverage the new severity configurations.

🔗 Analysis chain

Line range hint 1-464: Verify usage of the new severity field

While the SeverityArg has been properly integrated into the ScanArg struct, it appears that the rest of the file doesn't yet make use of this new information. Consider updating relevant functions (e.g., run_with_config or run_scan) to utilize the severity overrides provided by the user.

To ensure the new severity field is properly utilized, you can run the following script:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for usage of the severity field in relevant functions

# Test: Search for references to the severity field in key functions
rg -n 'fn (run_with_config|run_scan)' -A 20 -B 5 | rg 'severity'

# Test: Look for any new functions that might be handling the severity logic
rg -n 'fn.*severity'

Length of output: 147

crates/cli/src/utils/rule_overwrite.rs (5)

9-12: Consider making RuleOverwrite struct public

The RuleOverwrite struct is currently private. If you intend for it to be used outside of this module (e.g., in other parts of the crate or by other crates), consider adding the pub keyword to make it public.

Apply this diff to make the struct public:

-struct RuleOverwrite {
+pub struct RuleOverwrite {
   default_severity: Option<Severity>,
   by_rule_id: HashMap<String, Severity>,
 }

---

14-28: Add documentation comments to read_severity function

Adding documentation to the read_severity function will improve code readability and maintainability. It will help other developers understand the purpose and usage of this function.

Apply this diff to add documentation:

+/// Reads the severity and updates the severity mappings.
+///
+/// If `ids` is `None`, the function returns immediately.
+/// If `ids` is an empty vector, it sets the `default_severity`.
+/// Otherwise, it inserts each `id` with the given `severity` into `by_rule_id`.
 fn read_severity(
   severity: Severity,
   ids: &Option<Vec<String>>,
   by_rule_id: &mut HashMap<String, Severity>,
   default_severity: &mut Option<Severity>,
 ) {

---

46-54: Optimize severity retrieval in find method

In the find method, cloning occurs even when not necessary. Since Severity is typically a small enum, you can derive Copy for Severity to avoid cloning, making the code more efficient.

Apply this diff to derive Copy and adjust the method:

 // In ast_grep_config/src/lib.rs or wherever `Severity` is defined
 #[derive(Debug, Clone)]
+#[derive(Copy)]
 pub enum Severity {
   Error,
   Warning,
   Info,
 }

 // In the `find` method
 pub fn find(&self, id: &str) -> OverwriteResult {
   let severity = self
     .by_rule_id
     .get(id)
-    .cloned()
-    .or_else(|| self.default_severity.clone());
+    .copied()
+    .or(self.default_severity);
   OverwriteResult { severity }
 }

---

56-58: Evaluate the necessity of making OverwriteResult public

The OverwriteResult struct is declared as pub, and its field severity is also public. If OverwriteResult is only used within this module or crate, you may not need to make it public. Alternatively, consider providing accessor methods if external access is required while keeping fields encapsulated.

Apply this diff to adjust the visibility:

-pub struct OverwriteResult {
-  pub severity: Option<Severity>,
+struct OverwriteResult {
+  severity: Option<Severity>,
 }

If external access is needed, you can add a public method to retrieve the severity:

 impl OverwriteResult {
+  pub fn severity(&self) -> Option<Severity> {
+    self.severity
+  }

---

60-69: Remove unused generic parameter L in overwrite method

The generic parameter L in overwrite<L> is constrained by L: Language but is not used within the method body. This can be simplified by removing the generic parameter and using impl Language directly in the type.

Apply this diff to simplify the method signature:

 impl OverwriteResult {
-  pub fn overwrite<L>(&self, rule: &mut SerializableRuleConfig<L>)
-  where
-    L: Language,
+  pub fn overwrite(&self, rule: &mut SerializableRuleConfig<impl Language>)
   {
     if let Some(severity) = &self.severity {
       rule.severity = severity.clone();
     }
   }

Configuration used: CodeRabbit UI
Review profile: CHILL