mirror of
https://github.com/danbulant/oxc
synced 2026-05-19 04:08:41 +00:00
refactor(linter): LintContext can now only be constructed with a cfg enabled semantic. (#3761)
It has the same spirit as #3747 but with a much simpler approach. I've used the fact that @Boshen mentioned about linter always using CFG so now we assert `semantic.cfg().is_some()` in the `LintContext::new` because of this assertion we can have a `LintContext::cfg` that unwraps unchecked. Eliminates unnecessary checks in our hot paths. It has the best of both worlds, No complicated typing yet we still get the CFG as a non-optional value without extra ASM or branching.
This commit is contained in:
parent
4fb90eb009
commit
4d2b7f1227
11 changed files with 45 additions and 38 deletions
|
|
@ -287,6 +287,7 @@ impl IsolatedLintHandler {
|
|||
|
||||
let program = allocator.alloc(ret.program);
|
||||
let semantic_ret = SemanticBuilder::new(javascript_source_text, source_type)
|
||||
.with_cfg(true)
|
||||
.with_trivias(ret.trivias)
|
||||
.with_check_syntax_error(true)
|
||||
.build(program);
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
use std::{cell::RefCell, path::Path, rc::Rc, sync::Arc};
|
||||
|
||||
use oxc_cfg::ControlFlowGraph;
|
||||
use oxc_diagnostics::{OxcDiagnostic, Severity};
|
||||
use oxc_semantic::{AstNodes, JSDocFinder, ScopeTree, Semantic, SymbolTable};
|
||||
use oxc_span::{SourceType, Span};
|
||||
|
|
@ -34,7 +35,15 @@ pub struct LintContext<'a> {
|
|||
}
|
||||
|
||||
impl<'a> LintContext<'a> {
|
||||
/// # Panics
|
||||
/// If `semantic.cfg()` is `None`.
|
||||
pub fn new(file_path: Box<Path>, semantic: Rc<Semantic<'a>>) -> Self {
|
||||
// We should always check for `semantic.cfg()` being `Some` since we depend on it and it is
|
||||
// unwrapped without any runtime checks after construction.
|
||||
assert!(
|
||||
semantic.cfg().is_some(),
|
||||
"`LintContext` depends on `Semantic::cfg`, Build your semantic with cfg enabled(`SemanticBuilder::with_cfg`)."
|
||||
);
|
||||
let disable_directives =
|
||||
DisableDirectivesBuilder::new(semantic.source_text(), semantic.trivias().clone())
|
||||
.build();
|
||||
|
|
@ -78,6 +87,15 @@ impl<'a> LintContext<'a> {
|
|||
&self.semantic
|
||||
}
|
||||
|
||||
pub fn cfg(&self) -> &ControlFlowGraph {
|
||||
#[allow(unsafe_code)]
|
||||
// SAFETY: `LintContext::new` is the only way to construct a `LintContext` and we always
|
||||
// assert the existence of control flow so it should always be `Some`.
|
||||
unsafe {
|
||||
self.semantic().cfg().unwrap_unchecked()
|
||||
}
|
||||
}
|
||||
|
||||
pub fn disable_directives(&self) -> &DisableDirectives<'a> {
|
||||
&self.disable_directives
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ use oxc_ast::{
|
|||
AstKind,
|
||||
};
|
||||
use oxc_cfg::{
|
||||
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, InstructionKind,
|
||||
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, InstructionKind,
|
||||
ReturnInstructionKind,
|
||||
};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
|
|
@ -54,19 +54,16 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for GetterReturn {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
// https://eslint.org/docs/latest/rules/getter-return#handled_by_typescript
|
||||
if ctx.source_type().is_typescript() {
|
||||
return;
|
||||
}
|
||||
match node.kind() {
|
||||
AstKind::Function(func) if !func.is_typescript_syntax() => {
|
||||
self.run_diagnostic(node, ctx, cfg, func.span);
|
||||
self.run_diagnostic(node, ctx, func.span);
|
||||
}
|
||||
AstKind::ArrowFunctionExpression(expr) => {
|
||||
self.run_diagnostic(node, ctx, cfg, expr.span);
|
||||
self.run_diagnostic(node, ctx, expr.span);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
|
@ -180,17 +177,13 @@ impl GetterReturn {
|
|||
false
|
||||
}
|
||||
|
||||
fn run_diagnostic<'a>(
|
||||
&self,
|
||||
node: &AstNode<'a>,
|
||||
ctx: &LintContext<'a>,
|
||||
cfg: &ControlFlowGraph,
|
||||
span: Span,
|
||||
) {
|
||||
fn run_diagnostic<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>, span: Span) {
|
||||
if !Self::is_wanted_node(node, ctx) {
|
||||
return;
|
||||
}
|
||||
|
||||
let cfg = ctx.cfg();
|
||||
|
||||
let output = neighbors_filtered_by_edge_weight(
|
||||
cfg.graph(),
|
||||
node.cfg_id(),
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use oxc_cfg::{
|
|||
visit::{neighbors_filtered_by_edge_weight, EdgeRef},
|
||||
Direction,
|
||||
},
|
||||
BasicBlockId, ControlFlowGraph, EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
BasicBlockId, EdgeType, ErrorEdgeKind, InstructionKind,
|
||||
};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
|
|
@ -89,15 +89,13 @@ impl Rule for NoFallthrough {
|
|||
}
|
||||
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let AstKind::SwitchStatement(switch) = node.kind() else { return };
|
||||
|
||||
let cfg = ctx.cfg();
|
||||
let switch_id = node.cfg_id();
|
||||
let graph = cfg.graph();
|
||||
|
||||
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, cfg, node, switch);
|
||||
let (cfg_ids, tests, default, exit) = get_switch_semantic_cases(ctx, node, switch);
|
||||
|
||||
let Some(default_or_exit) = default.or(exit) else {
|
||||
// TODO: our `get_switch_semantic_cases` can't evaluate cfg_ids for switch statements
|
||||
|
|
@ -262,7 +260,6 @@ impl NoFallthrough {
|
|||
// Issue: <https://github.com/oxc-project/oxc/issues/3662>
|
||||
fn get_switch_semantic_cases(
|
||||
ctx: &LintContext,
|
||||
cfg: &ControlFlowGraph,
|
||||
node: &AstNode,
|
||||
switch: &SwitchStatement,
|
||||
) -> (
|
||||
|
|
@ -271,6 +268,7 @@ fn get_switch_semantic_cases(
|
|||
/* default */ Option<BasicBlockId>,
|
||||
/* exit */ Option<BasicBlockId>,
|
||||
) {
|
||||
let cfg = ctx.cfg();
|
||||
let graph = cfg.graph();
|
||||
let has_default = switch.cases.iter().any(SwitchCase::is_default_case);
|
||||
let (tests, exit) = graph
|
||||
|
|
|
|||
|
|
@ -56,9 +56,8 @@ enum DefinitelyCallsThisBeforeSuper {
|
|||
|
||||
impl Rule for NoThisBeforeSuper {
|
||||
fn run_once(&self, ctx: &LintContext) {
|
||||
let cfg = ctx.cfg();
|
||||
let semantic = ctx.semantic();
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
// first pass -> find super calls and local violations
|
||||
let mut wanted_nodes = Vec::new();
|
||||
|
|
|
|||
|
|
@ -31,11 +31,9 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for NoUnreachable {
|
||||
fn run_once(&self, ctx: &LintContext) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let nodes = ctx.nodes();
|
||||
let Some(root) = nodes.root_node() else { return };
|
||||
let cfg = ctx.cfg();
|
||||
let graph = cfg.graph();
|
||||
|
||||
// A pre-allocated vector containing the reachability status of all the basic blocks.
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use oxc_ast::{ast::Expression, AstKind};
|
||||
use oxc_cfg::{
|
||||
graph::visit::neighbors_filtered_by_edge_weight, ControlFlowGraph, EdgeType, Instruction,
|
||||
InstructionKind, ReturnInstructionKind,
|
||||
graph::visit::neighbors_filtered_by_edge_weight, EdgeType, Instruction, InstructionKind,
|
||||
ReturnInstructionKind,
|
||||
};
|
||||
use oxc_diagnostics::OxcDiagnostic;
|
||||
use oxc_macros::declare_oxc_lint;
|
||||
|
|
@ -50,9 +50,6 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for RequireRenderReturn {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
if !matches!(node.kind(), AstKind::ArrowFunctionExpression(_) | AstKind::Function(_)) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -66,7 +63,7 @@ impl Rule for RequireRenderReturn {
|
|||
return;
|
||||
}
|
||||
|
||||
if !contains_return_statement(node, cfg) {
|
||||
if !contains_return_statement(node, ctx) {
|
||||
match parent.kind() {
|
||||
AstKind::MethodDefinition(method) => {
|
||||
ctx.diagnostic(require_render_return_diagnostic(method.key.span()));
|
||||
|
|
@ -93,7 +90,8 @@ enum FoundReturn {
|
|||
const KEEP_WALKING_ON_THIS_PATH: bool = true;
|
||||
const STOP_WALKING_ON_THIS_PATH: bool = false;
|
||||
|
||||
fn contains_return_statement(node: &AstNode, cfg: &ControlFlowGraph) -> bool {
|
||||
fn contains_return_statement(node: &AstNode, ctx: &LintContext) -> bool {
|
||||
let cfg = ctx.cfg();
|
||||
let state = neighbors_filtered_by_edge_weight(
|
||||
cfg.graph(),
|
||||
node.cfg_id(),
|
||||
|
|
|
|||
|
|
@ -107,14 +107,14 @@ declare_oxc_lint!(
|
|||
|
||||
impl Rule for RulesOfHooks {
|
||||
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
// control flow dependant
|
||||
let Some(cfg) = ctx.semantic().cfg() else { return };
|
||||
|
||||
let AstKind::CallExpression(call) = node.kind() else { return };
|
||||
|
||||
if !is_react_hook(&call.callee) {
|
||||
return;
|
||||
}
|
||||
|
||||
let cfg = ctx.cfg();
|
||||
|
||||
let span = call.span;
|
||||
let hook_name =
|
||||
call.callee_name().expect("We identify hooks using their names so it should be named.");
|
||||
|
|
|
|||
|
|
@ -315,7 +315,8 @@ mod test {
|
|||
let source_type = SourceType::default();
|
||||
let parser_ret = Parser::new(&allocator, "", source_type).parse();
|
||||
let program = allocator.alloc(parser_ret.program);
|
||||
let semantic_ret = SemanticBuilder::new("", source_type).build(program).semantic;
|
||||
let semantic_ret =
|
||||
SemanticBuilder::new("", source_type).with_cfg(true).build(program).semantic;
|
||||
let semantic_ret = Rc::new(semantic_ret);
|
||||
|
||||
let path = Path::new("foo.js");
|
||||
|
|
|
|||
|
|
@ -101,19 +101,19 @@ pub fn declare_all_lint_rules(metadata: AllLintRulesMeta) -> TokenStream {
|
|||
}
|
||||
}
|
||||
|
||||
pub fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
pub(super) fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
|
||||
match self {
|
||||
#(Self::#struct_names(rule) => rule.run(node, ctx)),*
|
||||
}
|
||||
}
|
||||
|
||||
pub fn run_on_symbol<'a>(&self, symbol_id: SymbolId, ctx: &LintContext<'a>) {
|
||||
pub(super) fn run_on_symbol<'a>(&self, symbol_id: SymbolId, ctx: &LintContext<'a>) {
|
||||
match self {
|
||||
#(Self::#struct_names(rule) => rule.run_on_symbol(symbol_id, ctx)),*
|
||||
}
|
||||
}
|
||||
|
||||
pub fn run_once<'a>(&self, ctx: &LintContext<'a>) {
|
||||
pub(super) fn run_once<'a>(&self, ctx: &LintContext<'a>) {
|
||||
match self {
|
||||
#(Self::#struct_names(rule) => rule.run_once(ctx)),*
|
||||
}
|
||||
|
|
|
|||
|
|
@ -180,6 +180,7 @@ impl Oxc {
|
|||
let program = allocator.alloc(ret.program);
|
||||
|
||||
let semantic_ret = SemanticBuilder::new(source_text, source_type)
|
||||
.with_cfg(true)
|
||||
.with_trivias(ret.trivias.clone())
|
||||
.with_check_syntax_error(true)
|
||||
.build(program);
|
||||
|
|
|
|||
Loading…
Reference in a new issue