omnicoach 0.3.0

Lightweight, high-signal static analysis and command linter for Python and Rust
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121

//! Flags `contextlib.suppress` blocks.

use crate::code_lint::ast::ParsedFile;
use crate::code_lint::contract::{CodeRule, RuleTarget};
use crate::diagnostic::{Diagnostic, Language, RuleName, ViolationTemplate, violation_template};
use crate::rule_declaration::{
    Classification, Consensus, Declaration, EnforcementMode, Example, FilterListDefaults,
    ImpactedQuality, LanguageDefaults, ListKind, ListOption, Precision, Reference, RuleDoc,
    RuleOptions, Topic,
};
use std::collections::HashSet;
use std::path::Path;

const BANNED: ListOption = ListOption {
    kind: ListKind::Deny,
    doc: "Exception suppression calls flagged when called.",
    default: FilterListDefaults {
        base: &["suppress", "contextlib.suppress"],
        extend: &[],
        remove: &[],
    },
};

const TEMPLATE: ViolationTemplate = violation_template! {
    summary: "An exception is silenced with `{callee}()`.",
    rationale: "Silencing an exception without a recorded reason hides unexpected failures and leaves the next reader unable to tell intentional ignoring from accidental masking.",
    suggestion: "Replace the block with an explicit `except` handler.",
};

/// The rule's declaration.
pub const RULE: CodeRule<ListOption> = CodeRule {
    declaration: Declaration {
        name: RuleName("suppressed-exception"),
        template: &TEMPLATE,
        languages: &[Language::Python],
        options: RuleOptions {
            enforcement_mode: Some(LanguageDefaults::new(
                EnforcementMode::RequireExplanation,
                &[],
            )),
            options: BANNED,
        },
        classification: Classification {
            topics: &[Topic::ERROR_HANDLING],
            precision: Precision::Exact,
            consensus: Consensus::Opinionated,
            impacted_quality: ImpactedQuality::Maintainability,
        },
        doc: RuleDoc {
            summary: "Flags `contextlib.suppress` blocks.",
            what_it_does: indoc::indoc! {r"
            Flags `contextlib.suppress(...)` used as a context manager in a `with` statement."},
            why_is_this_bad: indoc::indoc! {r"
                `suppress` silently discards an exception. The code does not say why that failure is
                harmless, so a reader cannot tell an intentional ignore from a bug being hidden, and
                a later change that makes the exception meaningful goes unnoticed.

                An explicit `except` handler keeps the ignored case visible and can record it, for
                example with a debug log."},
            known_problems: None,
            references: &[
                Reference {
                    title: "Python docs: contextlib.suppress",
                    url: "https://docs.python.org/3/library/contextlib.html#contextlib.suppress",
                },
                Reference::NAME_RESOLUTION,
            ],
            examples: &[Example {
                language: Language::Python,
                flagged: indoc::indoc! {r"
                    with suppress(FileNotFoundError):
                        os.remove(lock_path)
                "},
                flagged_span: "suppress(FileNotFoundError)",
                fixed: indoc::indoc! {r#"
                    try:
                        os.remove(lock_path)
                    except FileNotFoundError:
                        logger.debug("Lock file %s was already removed by the cleanup job.", lock_path)
                "#},
            }],
        },
    },
    target: RuleTarget::All,
    check: check_file,
};

fn check_file(
    rule: &CodeRule<ListOption>,
    path: &Path,
    file: &ParsedFile,
    banned_calls: &HashSet<String>,
) -> Vec<Diagnostic> {
    rule.check_banned_calls_where(path, file, banned_calls, |matched| {
        matched.is_with_context_manager
    })
}

#[cfg(test)]
crate::test_utils::rule_test!(
    RULE,
    {
        Python => {
            pass: [
                suppress_call_outside_with_ignored => r#"
                    mgr = suppress(FileNotFoundError)
                "#,
            ],
            fail: [
                bare_suppress => r#"
                    with suppress(FileNotFoundError):
                        os.remove("tmp.txt")
                "# => "suppress(FileNotFoundError)",
                contextlib_qualified => r#"
                    with contextlib.suppress(KeyError):
                        data = cache["missing"]
                "# => "contextlib.suppress(KeyError)",
            ],
        },
    }
);