Skip to content

Commit

Permalink
Detect duplicate enum values
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Mar 22, 2024
1 parent 5223a09 commit c000951
Show file tree
Hide file tree
Showing 7 changed files with 112 additions and 29 deletions.
101 changes: 87 additions & 14 deletions src/analyzer/classlike_analyzer.rs
Original file line number Diff line number Diff line change
@@ -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>,
Expand All @@ -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();

Expand Down Expand Up @@ -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))
Expand All @@ -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);
}
}
}
}
}
_ => {}
}
Expand All @@ -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) {
Expand All @@ -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(),
);
}
6 changes: 1 addition & 5 deletions src/code_info/classlike_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -188,8 +187,6 @@ pub struct ClassLikeInfo {

pub attributes: Vec<AttributeInfo>,

pub enum_cases: Option<FxHashMap<String, EnumCaseInfo>>,

pub enum_type: Option<TAtomic>,
pub enum_constraint: Option<Box<TAtomic>>,

Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/code_info/issue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
pub enum IssueKind {
CannotInferGenericParam,
CustomIssue(String),
DuplicateEnumValue,
EmptyBlock,
FalsableReturnStatement,
FalseArgument,
Expand Down
2 changes: 1 addition & 1 deletion src/code_info_builder/classlike_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ pub(crate) fn scan(
*class_name,
&mut storage,
file_source.comments,
&file_source,
file_source,
user_defined,
);

Expand Down
16 changes: 7 additions & 9 deletions src/file_scanner_analyzer/scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions tests/inference/Enum/duplicateEnumValue/input.hack
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 2 additions & 0 deletions tests/inference/Enum/duplicateEnumValue/output.txt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit c000951

Please sign in to comment.