Skip to content

Commit

Permalink
Support delineating between prod and tests in call contexts
Browse files Browse the repository at this point in the history
  • Loading branch information
muglug committed Apr 2, 2024
1 parent d97cff5 commit 96f50d3
Show file tree
Hide file tree
Showing 13 changed files with 343 additions and 87 deletions.
6 changes: 3 additions & 3 deletions src/analyzer/dataflow/program_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ fn get_child_nodes(
generated_source.get_trace(interner, &config.root_dir)
);
new_issues.push(Issue::new(
IssueKind::TaintedData(t.clone()),
IssueKind::TaintedData(Box::new(t.clone())),
message,
**generated_source.pos.as_ref().unwrap(),
&None,
Expand Down Expand Up @@ -370,7 +370,7 @@ fn get_child_nodes(
generated_source.get_trace(interner, &config.root_dir)
);
new_issues.push(Issue::new(
IssueKind::TaintedData(t.clone()),
IssueKind::TaintedData(Box::new(t.clone())),
message,
**generated_source.pos.as_ref().unwrap(),
&None,
Expand Down Expand Up @@ -430,7 +430,7 @@ fn get_child_nodes(
new_destination.get_trace(interner, &config.root_dir)
);
new_issues.push(Issue::new(
IssueKind::TaintedData(matching_sink.clone()),
IssueKind::TaintedData(Box::new(matching_sink.clone())),
message,
**issue_pos,
&None,
Expand Down
38 changes: 38 additions & 0 deletions src/analyzer/expr/call/function_call_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,44 @@ pub(crate) fn analyze(
&functionlike_id,
);

if !function_storage.is_production_code {
if let Some(calling_context) = context.function_context.calling_functionlike_id {
let is_caller_production = match calling_context {
FunctionLikeIdentifier::Function(function_id) => {
codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
.unwrap()
.is_production_code
}
FunctionLikeIdentifier::Method(classlike_name, method_name) => {
codebase
.functionlike_infos
.get(&(classlike_name, method_name))
.unwrap()
.is_production_code
}
_ => false,
};

if is_caller_production {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::TestOnlyCall,
format!(
"Cannot call test-only function {} from non-test context",
statements_analyzer.get_interner().lookup(&name)
),
statements_analyzer.get_hpos(pos),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
)
}
}
}

let stmt_type = function_call_return_type_fetcher::fetch(
statements_analyzer,
expr,
Expand Down
38 changes: 38 additions & 0 deletions src/analyzer/expr/call_analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,44 @@ pub(crate) fn check_method_args(
check_template_result(statements_analyzer, template_result, pos, &functionlike_id);
}

if !functionlike_storage.is_production_code {
if let Some(calling_context) = context.function_context.calling_functionlike_id {
let is_caller_production = match calling_context {
FunctionLikeIdentifier::Function(function_id) => {
codebase
.functionlike_infos
.get(&(function_id, StrId::EMPTY))
.unwrap()
.is_production_code
}
FunctionLikeIdentifier::Method(classlike_name, method_name) => {
codebase
.functionlike_infos
.get(&(classlike_name, method_name))
.unwrap()
.is_production_code
}
_ => false,
};

if is_caller_production {
analysis_data.maybe_add_issue(
Issue::new(
IssueKind::TestOnlyCall,
format!(
"Cannot call test-only function {} from non-test context",
method_id.to_string(statements_analyzer.get_interner()),
),
statements_analyzer.get_hpos(pos),
&context.function_context.calling_functionlike_id,
),
statements_analyzer.get_config(),
statements_analyzer.get_file_path_actual(),
)
}
}
}

Ok(())
}

Expand Down
5 changes: 4 additions & 1 deletion src/code_info/classlike_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_hash::{FxHashMap, FxHashSet};

use crate::{
code_location::HPos, codebase_info::symbols::SymbolKind, functionlike_info::MetaStart,
t_atomic::TAtomic, t_union::TUnion, GenericParent,
issue::IssueKind, t_atomic::TAtomic, t_union::TUnion, GenericParent,
};
use indexmap::IndexMap;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -202,6 +202,8 @@ pub struct ClassLikeInfo {
pub namespace_position: Option<(usize, usize)>,

pub is_production_code: bool,

pub suppressed_issues: Vec<(IssueKind, HPos)>,
}

impl ClassLikeInfo {
Expand Down Expand Up @@ -270,6 +272,7 @@ impl ClassLikeInfo {
namespace_position: None,
is_production_code: true,
template_readonly: FxHashSet::default(),
suppressed_issues: vec![],
}
}
}
4 changes: 2 additions & 2 deletions src/code_info/functionlike_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub struct FunctionLikeInfo {

pub is_populated: bool,

pub suppressed_issues: Option<Vec<(IssueKind, HPos)>>,
pub suppressed_issues: Vec<(IssueKind, HPos)>,

pub deprecated: bool,

Expand Down Expand Up @@ -163,7 +163,7 @@ impl FunctionLikeInfo {
return_type_location: None,
is_populated: false,
user_defined: false,
suppressed_issues: None,
suppressed_issues: vec![],
deprecated: false,
template_types: vec![],
has_visitor_issues: false,
Expand Down
10 changes: 6 additions & 4 deletions src/code_info/issue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::{
#[derive(Clone, PartialEq, Eq, Hash, Display, Debug, Serialize, Deserialize, EnumString)]
pub enum IssueKind {
CannotInferGenericParam,
CustomIssue(String),
CustomIssue(Box<String>),
DuplicateEnumValue,
EmptyBlock,
FalsableReturnStatement,
Expand Down Expand Up @@ -85,6 +85,7 @@ pub enum IssueKind {
NullablePropertyAssignment,
NullableReturnStatement,
NullableReturnValue,
OnlyUsedInTests,
ParadoxicalCondition,
PossibleMethodCallOnNull,
PossiblyFalseArgument,
Expand All @@ -104,7 +105,8 @@ pub enum IssueKind {
RedundantNonnullTypeComparison,
RedundantTruthinessCheck,
RedundantTypeComparison,
TaintedData(SinkType),
TaintedData(Box<SinkType>),
TestOnlyCall,
UndefinedIntArrayOffset,
UndefinedStringArrayOffset,
UndefinedVariable,
Expand Down Expand Up @@ -147,7 +149,7 @@ impl IssueKind {
}

if all_custom_issues.contains(str) {
Ok(IssueKind::CustomIssue(str.to_string()))
Ok(IssueKind::CustomIssue(Box::new(str.to_string())))
} else {
Err(format!("Unknown issue {}", str))
}
Expand All @@ -156,7 +158,7 @@ impl IssueKind {
#[allow(clippy::inherent_to_string_shadow_display)]
pub fn to_string(&self) -> String {
match self {
Self::CustomIssue(str) => str.clone(),
Self::CustomIssue(str) => (**str).clone(),
//Self::TaintedData(sink_type) => format!("TaintedData({})", sink_type),
_ => format!("{}", self),
}
Expand Down
23 changes: 14 additions & 9 deletions src/code_info/symbol_references.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,15 +336,15 @@ impl SymbolReferences {
referenced_symbols_and_members
}

pub fn back_references(&self) -> FxHashMap<(StrId, StrId), FxHashSet<&(StrId, StrId)>> {
pub fn back_references(&self) -> FxHashMap<(StrId, StrId), FxHashSet<(StrId, StrId)>> {
let mut referenced_symbols_and_members = FxHashMap::default();

for (reference, symbol_references_to_symbols) in &self.symbol_references_to_symbols {
for r in symbol_references_to_symbols {
let v = referenced_symbols_and_members
.entry(*r)
.or_insert_with(FxHashSet::default);
v.insert(reference);
v.insert(*reference);
}
}

Expand All @@ -355,7 +355,7 @@ impl SymbolReferences {
let v = referenced_symbols_and_members
.entry(*r)
.or_insert_with(FxHashSet::default);
v.insert(reference);
v.insert(*reference);
}
}

Expand Down Expand Up @@ -404,13 +404,18 @@ impl SymbolReferences {
referenced_symbols_and_members
}

pub fn get_referenced_overridden_class_members(&self) -> FxHashSet<&(StrId, StrId)> {
let mut referenced_class_members = FxHashSet::default();
pub fn get_referenced_overridden_class_members(
&self,
) -> FxHashMap<(StrId, StrId), FxHashSet<(StrId, StrId)>> {
let mut referenced_class_members = FxHashMap::default();

for symbol_references_to_class_members in
self.symbol_references_to_overridden_members.values()
{
referenced_class_members.extend(symbol_references_to_class_members);
for (k, refs) in &self.symbol_references_to_overridden_members {
for r in refs {
let v = referenced_class_members
.entry(*r)
.or_insert_with(FxHashSet::default);
v.insert(*k);
}
}

referenced_class_members
Expand Down
12 changes: 10 additions & 2 deletions src/code_info_builder/classlike_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,13 @@ pub(crate) fn scan(
start_column: definition_location.start_column,
};

let mut suppressed_issues = vec![];

adjust_location_from_comments(
comments,
&mut meta_start,
file_source,
&mut vec![],
&mut suppressed_issues,
all_custom_issues,
);

Expand All @@ -81,7 +83,9 @@ pub(crate) fn scan(
storage.uses_position = uses_position;
storage.namespace_position = namespace_position;

storage.is_production_code = file_source.is_production_code;
storage.is_production_code &= file_source.is_production_code;

storage.suppressed_issues = suppressed_issues;

if !classlike_node.tparams.is_empty() {
let mut type_context = TypeResolutionContext {
Expand Down Expand Up @@ -582,6 +586,10 @@ pub(crate) fn scan(
storage.immutable = true;
}

if name == StrId::HAKANA_TEST_ONLY {
storage.is_production_code = false;
}

storage.attributes.push(AttributeInfo { name });

if name == StrId::SEALED {
Expand Down
9 changes: 5 additions & 4 deletions src/code_info_builder/functionlike_scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub(crate) fn scan_method(
user_defined,
);

functionlike_info.is_production_code = file_source.is_production_code;
functionlike_info.is_production_code &= file_source.is_production_code;

if classlike_name == StrId::BUILTIN_ENUM
&& (method_name == StrId::COERCE
Expand Down Expand Up @@ -362,6 +362,9 @@ pub(crate) fn get_functionlike(
StrId::HAKANA_SECURITY_ANALYSIS_IGNORE_PATH => {
functionlike_info.ignore_taint_path = true;
}
StrId::HAKANA_TEST_ONLY => {
functionlike_info.is_production_code = false;
}
StrId::HAKANA_SECURITY_ANALYSIS_IGNORE_PATH_IF_TRUE => {
functionlike_info.ignore_taints_if_true = true;
}
Expand Down Expand Up @@ -410,9 +413,7 @@ pub(crate) fn get_functionlike(
functionlike_info.name_location = Some(HPos::new(name_pos, file_source.file_path));
}

if !suppressed_issues.is_empty() {
functionlike_info.suppressed_issues = Some(suppressed_issues);
}
functionlike_info.suppressed_issues = suppressed_issues;

functionlike_info.is_async = fun_kind.is_async();
functionlike_info.effects = if let Some(contexts) = contexts {
Expand Down
17 changes: 12 additions & 5 deletions src/file_scanner_analyzer/scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,18 @@ pub fn scan_files(
group_size = 1;
}

let test_patterns = config
.test_files
.iter()
.map(|ignore_file| glob::Pattern::new(ignore_file).unwrap())
.collect::<Vec<_>>();
let mut test_patterns = vec![];

for ignore_file in &config.test_files {
match glob::Pattern::new(ignore_file) {
Ok(glob) => {
test_patterns.push(glob);
}
Err(error) => {
panic!("Parsing {} resulted in error {}", ignore_file, error);
}
}
}

for (i, str_path) in files_to_scan.into_iter().enumerate() {
let group = i % group_size;
Expand Down
Loading

0 comments on commit 96f50d3

Please sign in to comment.