From c000951cb3609727413bde5b7cfbcd2e39ff43f5 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Thu, 21 Mar 2024 23:15:55 -0400 Subject: [PATCH] Detect duplicate enum values --- src/analyzer/classlike_analyzer.rs | 101 +++++++++++++++--- src/code_info/classlike_info.rs | 6 +- src/code_info/issue.rs | 1 + src/code_info_builder/classlike_scanner.rs | 2 +- src/file_scanner_analyzer/scanner.rs | 16 ++- .../Enum/duplicateEnumValue/input.hack | 13 +++ .../Enum/duplicateEnumValue/output.txt | 2 + 7 files changed, 112 insertions(+), 29 deletions(-) create mode 100644 tests/inference/Enum/duplicateEnumValue/input.hack create mode 100644 tests/inference/Enum/duplicateEnumValue/output.txt diff --git a/src/analyzer/classlike_analyzer.rs b/src/analyzer/classlike_analyzer.rs index e861491e..58abb6c4 100644 --- a/src/analyzer/classlike_analyzer.rs +++ b/src/analyzer/classlike_analyzer.rs @@ -1,16 +1,19 @@ use crate::expression_analyzer; use crate::file_analyzer::FileAnalyzer; use crate::function_analysis_data::FunctionAnalysisData; -use crate::functionlike_analyzer::FunctionLikeAnalyzer; +use crate::functionlike_analyzer::{update_analysis_result_with_tast, FunctionLikeAnalyzer}; use crate::scope_analyzer::ScopeAnalyzer; use crate::scope_context::ScopeContext; use crate::statements_analyzer::StatementsAnalyzer; use crate::stmt_analyzer::AnalysisError; -use hakana_reflection_info::analysis_result::AnalysisResult; use hakana_reflection_info::codebase_info::symbols::SymbolKind; use hakana_reflection_info::data_flow::graph::DataFlowGraph; -use hakana_reflection_info::function_context::FunctionContext; +use hakana_reflection_info::function_context::{FunctionContext, FunctionLikeIdentifier}; +use hakana_reflection_info::issue::IssueKind; +use hakana_reflection_info::{analysis_result::AnalysisResult, issue::Issue}; +use hakana_str::StrId; use oxidized::aast; +use rustc_hash::FxHashMap; pub(crate) struct ClassLikeAnalyzer<'a> { file_analyzer: &'a FileAnalyzer<'a>, @@ -28,14 +31,15 @@ impl<'a> ClassLikeAnalyzer<'a> { analysis_result: &mut AnalysisResult, ) -> Result<(), AnalysisError> { let resolved_names = self.file_analyzer.resolved_names.clone(); - let name = if let Some(resolved_name) = resolved_names.get(&(stmt.name.0.start_offset() as u32)) { - *resolved_name - } else { - return Err(AnalysisError::InternalError( - format!("Cannot resolve class name {}", &stmt.name.1), - statements_analyzer.get_hpos(stmt.name.pos()), - )); - }; + let name = + if let Some(resolved_name) = resolved_names.get(&(stmt.name.0.start_offset() as u32)) { + *resolved_name + } else { + return Err(AnalysisError::InternalError( + format!("Cannot resolve class name {}", &stmt.name.1), + statements_analyzer.get_hpos(stmt.name.pos()), + )); + }; let codebase = self.file_analyzer.get_codebase(); @@ -88,6 +92,9 @@ impl<'a> ClassLikeAnalyzer<'a> { None, ); + let mut existing_enum_str_values = FxHashMap::default(); + let mut existing_enum_int_values = FxHashMap::default(); + for constant in &stmt.consts { match &constant.kind { aast::ClassConstKind::CCAbstract(Some(expr)) @@ -99,6 +106,46 @@ impl<'a> ClassLikeAnalyzer<'a> { &mut class_context, &mut None, )?; + + if codebase.enum_exists(&name) { + if let (Some(expr_value), Some(constant_name)) = ( + analysis_data.get_expr_type(expr.pos()), + resolved_names.get(&(constant.id.0.start_offset() as u32)), + ) { + if let Some(string_value) = expr_value.get_single_literal_string_value() + { + if let Some(existing_name) = + existing_enum_str_values.get(&string_value) + { + emit_dupe_enum_case_issue( + &mut analysis_data, + statements_analyzer, + name, + existing_name, + expr, + ); + } else { + existing_enum_str_values.insert(string_value, *constant_name); + } + } else if let Some(int_value) = + expr_value.get_single_literal_int_value() + { + if let Some(existing_name) = + existing_enum_int_values.get(&int_value) + { + emit_dupe_enum_case_issue( + &mut analysis_data, + statements_analyzer, + name, + existing_name, + expr, + ); + } else { + existing_enum_int_values.insert(int_value, *constant_name); + } + } + } + } } _ => {} } @@ -116,9 +163,12 @@ impl<'a> ClassLikeAnalyzer<'a> { } } - analysis_result - .symbol_references - .extend(analysis_data.symbol_references); + update_analysis_result_with_tast( + analysis_data, + analysis_result, + statements_analyzer.get_file_path(), + false, + ); for method in &stmt.methods { if method.abstract_ || matches!(classlike_storage.kind, SymbolKind::Interface) { @@ -132,3 +182,26 @@ impl<'a> ClassLikeAnalyzer<'a> { Ok(()) } } + +fn emit_dupe_enum_case_issue( + analysis_data: &mut FunctionAnalysisData, + statements_analyzer: &StatementsAnalyzer<'_>, + enum_name: StrId, + existing_name: &hakana_str::StrId, + expr: &aast::Expr<(), ()>, +) { + analysis_data.maybe_add_issue( + Issue::new( + IssueKind::DuplicateEnumValue, + format!( + "Duplicate enum value for {}, previously defined by case {}", + statements_analyzer.get_interner().lookup(&enum_name), + statements_analyzer.get_interner().lookup(existing_name) + ), + statements_analyzer.get_hpos(expr.pos()), + &Some(FunctionLikeIdentifier::Function(enum_name)), + ), + statements_analyzer.get_config(), + statements_analyzer.get_file_path_actual(), + ); +} diff --git a/src/code_info/classlike_info.rs b/src/code_info/classlike_info.rs index 85d25d0e..d159ac3f 100644 --- a/src/code_info/classlike_info.rs +++ b/src/code_info/classlike_info.rs @@ -11,8 +11,7 @@ use indexmap::IndexMap; use serde::{Deserialize, Serialize}; use crate::{ - attribute_info::AttributeInfo, class_constant_info::ConstantInfo, enum_case_info::EnumCaseInfo, - property_info::PropertyInfo, + attribute_info::AttributeInfo, class_constant_info::ConstantInfo, property_info::PropertyInfo, }; #[derive(Clone, Debug, Serialize, Deserialize)] @@ -188,8 +187,6 @@ pub struct ClassLikeInfo { pub attributes: Vec, - pub enum_cases: Option>, - pub enum_type: Option, pub enum_constraint: Option>, @@ -242,7 +239,6 @@ impl ClassLikeInfo { direct_parent_interfaces: vec![], required_classlikes: vec![], inheritable_method_ids: FxHashMap::default(), - enum_cases: None, enum_type: None, enum_constraint: None, hash: None, diff --git a/src/code_info/issue.rs b/src/code_info/issue.rs index c39f4004..b8e17900 100644 --- a/src/code_info/issue.rs +++ b/src/code_info/issue.rs @@ -15,6 +15,7 @@ use crate::{ pub enum IssueKind { CannotInferGenericParam, CustomIssue(String), + DuplicateEnumValue, EmptyBlock, FalsableReturnStatement, FalseArgument, diff --git a/src/code_info_builder/classlike_scanner.rs b/src/code_info_builder/classlike_scanner.rs index f1b3577e..2c927d2b 100644 --- a/src/code_info_builder/classlike_scanner.rs +++ b/src/code_info_builder/classlike_scanner.rs @@ -639,7 +639,7 @@ pub(crate) fn scan( *class_name, &mut storage, file_source.comments, - &file_source, + file_source, user_defined, ); diff --git a/src/file_scanner_analyzer/scanner.rs b/src/file_scanner_analyzer/scanner.rs index 004f815f..d9190726 100644 --- a/src/file_scanner_analyzer/scanner.rs +++ b/src/file_scanner_analyzer/scanner.rs @@ -613,15 +613,13 @@ fn invalidate_changed_codebase_elements( Some(kind) => { if let SymbolKind::TypeDefinition = kind { codebase.type_definitions.remove(&ast_node.name); - } else { - if let Some(classlike_info) = - codebase.classlike_infos.remove(&ast_node.name) - { - for method_name in classlike_info.methods { - codebase - .functionlike_infos - .remove(&(ast_node.name, method_name)); - } + } else if let Some(classlike_info) = + codebase.classlike_infos.remove(&ast_node.name) + { + for method_name in classlike_info.methods { + codebase + .functionlike_infos + .remove(&(ast_node.name, method_name)); } } } diff --git a/tests/inference/Enum/duplicateEnumValue/input.hack b/tests/inference/Enum/duplicateEnumValue/input.hack new file mode 100644 index 00000000..b3973783 --- /dev/null +++ b/tests/inference/Enum/duplicateEnumValue/input.hack @@ -0,0 +1,13 @@ +enum Suit: string { + Hearts = "h"; + Diamonds = "d"; + Clubs = "c"; + Spades = "c"; +} + +enum Color: int { + Red = 0; + Green = 1; + Blue = 2; + Yellow = 2; +} \ No newline at end of file diff --git a/tests/inference/Enum/duplicateEnumValue/output.txt b/tests/inference/Enum/duplicateEnumValue/output.txt new file mode 100644 index 00000000..9ea47240 --- /dev/null +++ b/tests/inference/Enum/duplicateEnumValue/output.txt @@ -0,0 +1,2 @@ +ERROR: DuplicateEnumValue - input.hack:5:14 - Duplicate enum value for Suit, previously defined by case Clubs +ERROR: DuplicateEnumValue - input.hack:12:14 - Duplicate enum value for Color, previously defined by case Blue