Support check preferred-local-alias

Author: CppCXY

crates/emmylua_code_analysis/src/compilation/analyzer/common/mod.rs

@@ -18,11 +18,11 @@ pub fn bind_type(
         // type backward
         if type_cache.is_infer() {
             if let LuaTypeOwner::Decl(decl_id) = &type_owner {
-                if let Some(refs) = db
+                if let Some(decl_ref) = db
                     .get_reference_index()
                     .get_decl_references(&decl_id.file_id, decl_id)
                 {
-                    if refs.iter().any(|it| it.is_write) {
+                    if decl_ref.mutable {
                         match &type_cache.as_type() {
                             LuaType::IntegerConst(_) => {
                                 type_cache = LuaTypeCache::InferType(LuaType::Integer)

crates/emmylua_code_analysis/src/compilation/analyzer/doc/type_def_tags.rs

@@ -245,11 +245,11 @@ fn get_local_stat_reference_ranges(
     let first_local = local_stat.child::<LuaLocalName>()?;
     let decl_id = LuaDeclId::new(file_id, first_local.get_position());
     let mut ranges = Vec::new();
-    let refs = analyzer
+    let decl_ref = analyzer
         .db
         .get_reference_index_mut()
         .get_decl_references(&file_id, &decl_id)?;
-    for decl_ref in refs {
+    for decl_ref in &decl_ref.cells {
         let syntax_id = LuaSyntaxId::new(LuaSyntaxKind::NameExpr.into(), decl_ref.range.clone());
         let name_node = syntax_id.to_node_from_root(&analyzer.root)?;
         if let Some(parent1) = name_node.parent() {

crates/emmylua_code_analysis/src/compilation/analyzer/flow/bind_analyze/stats.rs

@@ -44,21 +44,15 @@ pub fn bind_local_stat(
 }
 
 fn check_local_immutable(binder: &mut FlowBinder, decl_id: LuaDeclId) -> bool {
-    let Some(refs) = binder
+    let Some(decl_ref) = binder
         .db
         .get_reference_index()
         .get_decl_references(&binder.file_id, &decl_id)
     else {
         return true;
     };
 
-    for r in refs {
-        if r.is_write {
-            return false;
-        }
-    }
-
-    true
+    !decl_ref.mutable
 }
 
 fn check_value_expr_is_check_expr(value_expr: LuaExpr) -> bool {

crates/emmylua_code_analysis/src/db_index/reference/file_reference.rs

@@ -5,7 +5,7 @@ use crate::db_index::LuaDeclId;
 
 #[derive(Debug)]
 pub struct FileReference {
-    decl_references: HashMap<LuaDeclId, Vec<DeclReference>>,
+    decl_references: HashMap<LuaDeclId, DeclReference>,
     references_to_decl: HashMap<TextRange, LuaDeclId>,
 }
 
@@ -23,19 +23,19 @@ impl FileReference {
         }
 
         self.references_to_decl.insert(range, decl_id);
-        let decl_ref = DeclReference { range, is_write };
+        let decl_ref = DeclReferenceCell { range, is_write };
 
         self.decl_references
             .entry(decl_id)
-            .or_insert_with(Vec::new)
-            .push(decl_ref);
+            .or_insert_with(DeclReference::new)
+            .add_cell(decl_ref);
     }
 
-    pub fn get_decl_references(&self, decl_id: &LuaDeclId) -> Option<&Vec<DeclReference>> {
+    pub fn get_decl_references(&self, decl_id: &LuaDeclId) -> Option<&DeclReference> {
         self.decl_references.get(decl_id)
     }
 
-    pub fn get_decl_references_map(&self) -> &HashMap<LuaDeclId, Vec<DeclReference>> {
+    pub fn get_decl_references_map(&self) -> &HashMap<LuaDeclId, DeclReference> {
         &self.decl_references
     }
 
@@ -45,7 +45,30 @@ impl FileReference {
 }
 
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-pub struct DeclReference {
+pub struct DeclReferenceCell {
     pub range: TextRange,
     pub is_write: bool,
 }
+
+#[derive(Debug, Clone, Eq, PartialEq)]
+pub struct DeclReference {
+    pub cells: Vec<DeclReferenceCell>,
+    pub mutable: bool,
+}
+
+impl DeclReference {
+    pub fn new() -> Self {
+        Self {
+            cells: Vec::new(),
+            mutable: false,
+        }
+    }
+
+    pub fn add_cell(&mut self, cell: DeclReferenceCell) {
+        if cell.is_write {
+            self.mutable = true;
+        }
+
+        self.cells.push(cell);
+    }
+}

crates/emmylua_code_analysis/src/db_index/reference/mod.rs

@@ -4,12 +4,12 @@ mod string_reference;
 use std::collections::{HashMap, HashSet};
 
 use emmylua_parser::LuaSyntaxId;
-pub use file_reference::{DeclReference, FileReference};
+pub use file_reference::{DeclReferenceCell, FileReference};
 use rowan::TextRange;
 use smol_str::SmolStr;
 use string_reference::StringReference;
 
-use crate::{FileId, InFiled};
+use crate::{FileId, InFiled, db_index::reference::file_reference::DeclReference};
 
 use super::{LuaDeclId, LuaMemberKey, LuaTypeDeclId, traits::LuaIndex};
 
@@ -105,7 +105,7 @@ impl LuaReferenceIndex {
         &self,
         file_id: &FileId,
         decl_id: &LuaDeclId,
-    ) -> Option<&Vec<DeclReference>> {
+    ) -> Option<&DeclReference> {
         self.file_references
             .get(file_id)?
             .get_decl_references(decl_id)
@@ -118,7 +118,7 @@ impl LuaReferenceIndex {
     pub fn get_decl_references_map(
         &self,
         file_id: &FileId,
-    ) -> Option<&HashMap<LuaDeclId, Vec<DeclReference>>> {
+    ) -> Option<&HashMap<LuaDeclId, DeclReference>> {
         self.file_references
             .get(file_id)
             .map(|file_reference| file_reference.get_decl_references_map())

crates/emmylua_code_analysis/src/diagnostic/checker/code_style/mod.rs

@@ -1 +1,2 @@
 pub mod non_literal_expressions_in_assert;
+pub mod preferred_local_alias;

crates/emmylua_code_analysis/src/diagnostic/checker/code_style/preferred_local_alias.rs

@@ -0,0 +1,231 @@
+use std::collections::{HashMap, HashSet};
+
+use emmylua_parser::{
+    LuaAst, LuaAstNode, LuaAstToken, LuaExpr, LuaIndexExpr, LuaIndexKey, LuaLocalStat,
+    LuaSyntaxKind,
+};
+use rowan::{NodeOrToken, TextRange};
+
+use crate::{
+    DiagnosticCode, LuaDeclId, LuaSemanticDeclId, SemanticDeclLevel, SemanticModel,
+    diagnostic::checker::{Checker, DiagnosticContext},
+};
+
+pub struct PreferredLocalAliasChecker;
+
+impl Checker for PreferredLocalAliasChecker {
+    const CODES: &[DiagnosticCode] = &[DiagnosticCode::PreferredLocalAlias];
+
+    fn check(context: &mut DiagnosticContext, semantic_model: &SemanticModel) {
+        let mut local_alias_set = LocalAliasSet::new();
+        let root = semantic_model.get_root().clone();
+        for walk in root.walk_descendants::<LuaAst>() {
+            match walk {
+                rowan::WalkEvent::Enter(node) => {
+                    if is_scope(&node) {
+                        local_alias_set.push();
+                    }
+
+                    match node {
+                        LuaAst::LuaLocalStat(local_stat) => {
+                            collect_local_alias(&mut local_alias_set, semantic_model, &local_stat);
+                        }
+                        LuaAst::LuaIndexExpr(index_expr) => {
+                            check_index_expr_preference(
+                                context,
+                                &local_alias_set,
+                                semantic_model,
+                                &index_expr,
+                            );
+                        }
+                        _ => {}
+                    }
+                }
+                rowan::WalkEvent::Leave(node) => {
+                    if is_scope(&node) {
+                        local_alias_set.pop();
+                    }
+                }
+            }
+        }
+    }
+}
+
+fn is_scope(node: &LuaAst) -> bool {
+    matches!(
+        node.syntax().kind().into(),
+        LuaSyntaxKind::Chunk | LuaSyntaxKind::Block | LuaSyntaxKind::ClosureExpr
+    )
+}
+
+fn collect_local_alias(
+    local_alias_set: &mut LocalAliasSet,
+    semantic_model: &SemanticModel,
+    local_stat: &LuaLocalStat,
+) -> Option<()> {
+    let local_list = local_stat.get_local_name_list().collect::<Vec<_>>();
+    let value_expr = local_stat.get_value_exprs().collect::<Vec<_>>();
+    let min_len = local_list.len().min(value_expr.len());
+    for i in 0..min_len {
+        let local_name = &local_list[i];
+        let value_expr = &value_expr[i];
+        if is_only_dot_index_expr(value_expr).unwrap_or(false) {
+            let decl_id = LuaDeclId::new(semantic_model.get_file_id(), local_name.get_position());
+            let decl_refs = semantic_model
+                .get_db()
+                .get_reference_index()
+                .get_decl_references(&semantic_model.get_file_id(), &decl_id);
+            if let Some(decl_refs) = decl_refs {
+                if decl_refs.mutable {
+                    continue;
+                }
+            }
+
+            let name = match value_expr {
+                LuaExpr::IndexExpr(index_expr) => {
+                    let index_key = index_expr.get_index_key()?;
+                    match index_key {
+                        LuaIndexKey::Name(name_token) => name_token.get_name_text().to_string(),
+                        _ => continue,
+                    }
+                }
+                _ => continue,
+            };
+            let node_or_token = NodeOrToken::Node(value_expr.syntax().clone());
+            if let Some(semantic_id) =
+                semantic_model.find_decl(node_or_token, SemanticDeclLevel::NoTrace)
+            {
+                let name_token = local_name.get_name_token()?;
+                let preferred_name = name_token.get_name_text();
+
+                local_alias_set.insert(name, preferred_name.to_string(), semantic_id);
+                local_alias_set.add_disable_check(value_expr.get_range());
+            }
+        }
+    }
+
+    Some(())
+}
+
+fn is_only_dot_index_expr(expr: &LuaExpr) -> Option<bool> {
+    let mut index_expr = match expr {
+        LuaExpr::IndexExpr(index_expr) => index_expr.clone(),
+        _ => return Some(false),
+    };
+
+    loop {
+        let index_token = index_expr.get_index_token()?;
+        if !index_token.is_dot() {
+            return Some(false);
+        }
+        match index_expr.get_prefix_expr() {
+            Some(LuaExpr::NameExpr(_)) => return Some(true),
+            Some(LuaExpr::IndexExpr(prefix_index_expr)) => {
+                index_expr = prefix_index_expr;
+            }
+            _ => return Some(false),
+        }
+    }
+}
+
+#[derive(Debug)]
+struct LocalAliasSet {
+    local_alias_stack: Vec<HashMap<String, (LuaSemanticDeclId, String)>>,
+    disable_check: HashSet<TextRange>,
+}
+
+impl LocalAliasSet {
+    fn new() -> Self {
+        LocalAliasSet {
+            local_alias_stack: vec![HashMap::new()],
+            disable_check: HashSet::new(),
+        }
+    }
+
+    fn push(&mut self) {
+        self.local_alias_stack.push(HashMap::new());
+    }
+
+    fn pop(&mut self) {
+        self.local_alias_stack.pop();
+    }
+
+    fn insert(&mut self, name: String, preferred_name: String, decl_id: LuaSemanticDeclId) {
+        if let Some(map) = self.local_alias_stack.last_mut() {
+            map.insert(name, (decl_id, preferred_name));
+        }
+    }
+
+    fn get(&self, name: &str) -> Option<(LuaSemanticDeclId, String)> {
+        for map in self.local_alias_stack.iter().rev() {
+            if let Some(item) = map.get(name) {
+                return Some(item.clone());
+            }
+        }
+        None
+    }
+
+    fn add_disable_check(&mut self, range: TextRange) {
+        self.disable_check.insert(range);
+    }
+
+    fn is_disable_check(&self, range: &TextRange) -> bool {
+        self.disable_check.contains(range)
+    }
+}
+
+fn check_index_expr_preference(
+    context: &mut DiagnosticContext,
+    local_alias_set: &LocalAliasSet,
+    semantic_model: &SemanticModel,
+    index_expr: &LuaIndexExpr,
+) -> Option<()> {
+    if local_alias_set.is_disable_check(&index_expr.get_range()) {
+        return Some(());
+    }
+
+    let expr = LuaExpr::IndexExpr(index_expr.clone());
+    if !is_only_dot_index_expr(&expr).unwrap_or(false) {
+        return Some(());
+    }
+
+    let parent = index_expr.get_parent::<LuaAst>()?;
+    match parent {
+        LuaAst::LuaAssignStat(assign_stat) => {
+            let eq = assign_stat.get_assign_op()?;
+            if eq.get_position() > index_expr.get_position() {
+                return Some(());
+            }
+        }
+        LuaAst::LuaFuncStat(_) => {
+            return Some(());
+        }
+        _ => {}
+    }
+
+    let index_key = index_expr.get_index_key()?;
+    let name = match index_key {
+        LuaIndexKey::Name(name_token) => name_token.get_name_text().to_string(),
+        _ => return Some(()),
+    };
+
+    let (semantic_id, preferred_name) = local_alias_set.get(&name)?;
+    if semantic_model.is_reference_to(
+        index_expr.syntax().clone(),
+        semantic_id,
+        SemanticDeclLevel::NoTrace,
+    ) {
+        context.add_diagnostic(
+            DiagnosticCode::PreferredLocalAlias,
+            index_expr.get_range(),
+            t!(
+                "Prefer use local alias variable '%{name}'",
+                name = preferred_name
+            )
+            .to_string(),
+            None,
+        );
+    }
+
+    Some(())
+}

crates/emmylua_code_analysis/src/diagnostic/checker/local_const_reassign.rs

@@ -44,7 +44,7 @@ fn check_local_const_reassign(
     let refs_index = semantic_model.get_db().get_reference_index();
     let local_refs = refs_index.get_local_reference(&file_id)?;
     let decl_refs = local_refs.get_decl_references(decl_id)?;
-    for decl_ref in decl_refs {
+    for decl_ref in &decl_refs.cells {
         if decl_ref.is_write {
             match attrib {
                 LocalAttribute::Const => {