From eb4ba4af60dd45feb9a3f0b32e1a6cb1f23b7fa7 Mon Sep 17 00:00:00 2001 From: Matt Brown Date: Mon, 18 Mar 2024 13:40:28 -0400 Subject: [PATCH] Report parser error for invalid file --- src/aast_utils/lib.rs | 8 +-- src/code_info/codebase_info/mod.rs | 4 +- src/code_info/file_info.rs | 11 +++- src/code_info_builder/lib.rs | 2 +- src/file_scanner_analyzer/analyzer.rs | 2 +- src/file_scanner_analyzer/lib.rs | 55 +++++++++++++------ src/file_scanner_analyzer/scanner.rs | 29 ++++++---- src/file_scanner_analyzer/wasm.rs | 3 +- .../type_comparator/atomic_type_comparator.rs | 7 ++- .../fileWithGitMarkersIncluded/output.txt | 2 +- .../output.txt | 2 +- 11 files changed, 80 insertions(+), 45 deletions(-) diff --git a/src/aast_utils/lib.rs b/src/aast_utils/lib.rs index 4c075f7e..e483f0d1 100644 --- a/src/aast_utils/lib.rs +++ b/src/aast_utils/lib.rs @@ -1,6 +1,7 @@ use aast_parser::rust_aast_parser_types::Env as AastParserEnv; use hakana_reflection_info::code_location::{FilePath, HPos}; +use hakana_reflection_info::file_info::ParserError; use hakana_str::{StrId, ThreadedInterner}; use name_context::NameContext; use naming_visitor::Scanner; @@ -17,13 +18,6 @@ use std::sync::Arc; pub mod name_context; mod naming_visitor; -#[derive(Debug)] -pub enum ParserError { - CannotReadFile, - NotAHackFile, - SyntaxError { message: String, pos: HPos }, -} - pub fn get_aast_for_path_and_contents( file_path: FilePath, file_path_str: &str, diff --git a/src/code_info/codebase_info/mod.rs b/src/code_info/codebase_info/mod.rs index 521fe73d..152809f2 100644 --- a/src/code_info/codebase_info/mod.rs +++ b/src/code_info/codebase_info/mod.rs @@ -10,8 +10,8 @@ use crate::property_info::PropertyInfo; use crate::t_atomic::TAtomic; use crate::t_union::TUnion; use crate::type_definition_info::TypeDefinitionInfo; -use hakana_str::StrId; use crate::{class_constant_info::ConstantInfo, code_location::FilePath}; +use hakana_str::StrId; use rustc_hash::{FxHashMap, FxHashSet}; use serde::{Deserialize, Serialize}; @@ -396,7 +396,7 @@ impl CodebaseInfo { pub fn has_invalid_file(&self, file_path: &FilePath) -> bool { if let Some(file_info) = self.files.get(file_path) { - !file_info.valid_file + file_info.parser_error.is_some() } else { false } diff --git a/src/code_info/file_info.rs b/src/code_info/file_info.rs index 32030d88..af614cc4 100644 --- a/src/code_info/file_info.rs +++ b/src/code_info/file_info.rs @@ -1,10 +1,17 @@ use serde::{Deserialize, Serialize}; -use crate::ast_signature::DefSignatureNode; +use crate::{ast_signature::DefSignatureNode, code_location::HPos}; + +#[derive(Debug, Serialize, Deserialize, Clone)] +pub enum ParserError { + CannotReadFile, + NotAHackFile, + SyntaxError { message: String, pos: HPos }, +} #[derive(Clone, Serialize, Deserialize, Debug, Default)] pub struct FileInfo { pub ast_nodes: Vec, pub closure_refs: Vec, - pub valid_file: bool, + pub parser_error: Option, } diff --git a/src/code_info_builder/lib.rs b/src/code_info_builder/lib.rs index c33b1278..62c28e8b 100644 --- a/src/code_info_builder/lib.rs +++ b/src/code_info_builder/lib.rs @@ -874,7 +874,7 @@ pub fn collect_info_for_aast( FileInfo { closure_refs: checker.closure_refs, ast_nodes: checker.ast_nodes, - valid_file: true, + parser_error: None, }, ); } diff --git a/src/file_scanner_analyzer/analyzer.rs b/src/file_scanner_analyzer/analyzer.rs index 01a76657..72dc7af7 100644 --- a/src/file_scanner_analyzer/analyzer.rs +++ b/src/file_scanner_analyzer/analyzer.rs @@ -1,11 +1,11 @@ use crate::{get_aast_for_path, update_progressbar, SuccessfulScanData}; -use hakana_aast_helper::ParserError; use hakana_analyzer::config::Config; use hakana_analyzer::file_analyzer; use hakana_logger::Logger; use hakana_reflection_info::analysis_result::AnalysisResult; use hakana_reflection_info::code_location::{FilePath, HPos}; use hakana_reflection_info::codebase_info::CodebaseInfo; +use hakana_reflection_info::file_info::ParserError; use hakana_reflection_info::issue::{Issue, IssueKind}; use hakana_reflection_info::symbol_references::SymbolReferences; use hakana_reflection_info::FileSource; diff --git a/src/file_scanner_analyzer/lib.rs b/src/file_scanner_analyzer/lib.rs index 18964ca4..a1e6098a 100644 --- a/src/file_scanner_analyzer/lib.rs +++ b/src/file_scanner_analyzer/lib.rs @@ -3,7 +3,7 @@ pub(crate) mod populator; use analyzer::analyze_files; use diff::{mark_safe_symbols_from_diff, CachedAnalysis}; use file::{FileStatus, VirtualFileSystem}; -use hakana_aast_helper::{get_aast_for_path_and_contents, ParserError}; +use hakana_aast_helper::get_aast_for_path_and_contents; use hakana_analyzer::config::Config; use hakana_analyzer::dataflow::program_analyzer::{find_connections, find_tainted_data}; use hakana_logger::Logger; @@ -11,6 +11,7 @@ use hakana_reflection_info::analysis_result::AnalysisResult; use hakana_reflection_info::code_location::{FilePath, HPos}; use hakana_reflection_info::codebase_info::CodebaseInfo; use hakana_reflection_info::data_flow::graph::{GraphKind, WholeProgramKind}; +use hakana_reflection_info::file_info::ParserError; use hakana_reflection_info::issue::{Issue, IssueKind}; use hakana_reflection_info::symbol_references::SymbolReferences; use hakana_str::{Interner, StrId}; @@ -446,24 +447,44 @@ fn update_progressbar(percentage: u64, bar: Option>) { fn add_invalid_files(scan_data: &SuccessfulScanData, analysis_result: &mut AnalysisResult) { for (file_path, file_info) in &scan_data.codebase.files { - if !file_info.valid_file { + if let Some(parser_error) = &file_info.parser_error { analysis_result.emitted_issues.insert( *file_path, - vec![Issue::new( - IssueKind::InvalidHackFile, - "Invalid Hack file".to_string(), - HPos { - file_path: *file_path, - start_offset: 1, - end_offset: 1, - start_line: 1, - end_line: 1, - start_column: 1, - end_column: 1, - insertion_start: None, - }, - &None, - )], + vec![match parser_error { + ParserError::NotAHackFile => Issue::new( + IssueKind::InvalidHackFile, + "Invalid Hack file".to_string(), + HPos { + file_path: *file_path, + start_offset: 0, + end_offset: 0, + start_line: 0, + end_line: 0, + start_column: 0, + end_column: 0, + insertion_start: None, + }, + &None, + ), + ParserError::CannotReadFile => Issue::new( + IssueKind::InvalidHackFile, + "Cannot read file".to_string(), + HPos { + file_path: *file_path, + start_offset: 0, + end_offset: 0, + start_line: 0, + end_line: 0, + start_column: 0, + end_column: 0, + insertion_start: None, + }, + &None, + ), + ParserError::SyntaxError { message, pos } => { + Issue::new(IssueKind::InvalidHackFile, message.clone(), *pos, &None) + } + }], ); } } diff --git a/src/file_scanner_analyzer/scanner.rs b/src/file_scanner_analyzer/scanner.rs index f8d56eb8..86bb263e 100644 --- a/src/file_scanner_analyzer/scanner.rs +++ b/src/file_scanner_analyzer/scanner.rs @@ -20,7 +20,6 @@ use crate::get_aast_for_path; use crate::SuccessfulScanData; use ast_differ::get_diff; use hakana_aast_helper::name_context::NameContext; -use hakana_aast_helper::ParserError; use hakana_analyzer::config::Config; use hakana_logger::Logger; use hakana_reflection_info::code_location::FilePath; @@ -28,6 +27,7 @@ use hakana_reflection_info::codebase_info::symbols::SymbolKind; use hakana_reflection_info::codebase_info::CodebaseInfo; use hakana_reflection_info::diff::CodebaseDiff; use hakana_reflection_info::file_info::FileInfo; +use hakana_reflection_info::file_info::ParserError; use hakana_reflection_info::FileSource; use hakana_str::Interner; use hakana_str::StrId; @@ -306,7 +306,7 @@ pub fn scan_files( .lookup(&file_path.0) .to_string(); - if let Ok(scanner_result) = scan_file( + match scan_file( &str_path, **file_path, &config.all_custom_issues, @@ -317,14 +317,23 @@ pub fn scan_files( !test_patterns.iter().any(|p| p.matches(&str_path)), &logger, ) { - resolved_names - .lock() - .unwrap() - .insert(**file_path, scanner_result); - } else { - resolved_names.lock().unwrap().remove(*file_path); - new_codebase.files.insert(**file_path, FileInfo::default()); - invalid_files.lock().unwrap().push(**file_path); + Ok(scanner_result) => { + resolved_names + .lock() + .unwrap() + .insert(**file_path, scanner_result); + } + Err(parser_error) => { + resolved_names.lock().unwrap().remove(*file_path); + new_codebase.files.insert( + **file_path, + FileInfo { + parser_error: Some(parser_error), + ..FileInfo::default() + }, + ); + invalid_files.lock().unwrap().push(**file_path); + } } update_progressbar(i as u64, bar.clone()); diff --git a/src/file_scanner_analyzer/wasm.rs b/src/file_scanner_analyzer/wasm.rs index 958614b9..b9847cab 100644 --- a/src/file_scanner_analyzer/wasm.rs +++ b/src/file_scanner_analyzer/wasm.rs @@ -1,5 +1,5 @@ +use hakana_aast_helper::get_aast_for_path_and_contents; use hakana_aast_helper::name_context::NameContext; -use hakana_aast_helper::{get_aast_for_path_and_contents, ParserError}; use hakana_analyzer::config::Config; use hakana_analyzer::dataflow::program_analyzer::find_tainted_data; use hakana_analyzer::file_analyzer; @@ -8,6 +8,7 @@ use hakana_reflection_info::analysis_result::AnalysisResult; use hakana_reflection_info::code_location::{FilePath, HPos}; use hakana_reflection_info::codebase_info::CodebaseInfo; use hakana_reflection_info::data_flow::graph::{GraphKind, WholeProgramKind}; +use hakana_reflection_info::file_info::ParserError; use hakana_reflection_info::issue::{Issue, IssueKind}; use hakana_reflection_info::symbol_references::SymbolReferences; use hakana_reflection_info::FileSource; diff --git a/src/ttype/type_comparator/atomic_type_comparator.rs b/src/ttype/type_comparator/atomic_type_comparator.rs index e8770ea8..31287bc3 100644 --- a/src/ttype/type_comparator/atomic_type_comparator.rs +++ b/src/ttype/type_comparator/atomic_type_comparator.rs @@ -798,8 +798,11 @@ pub(crate) fn can_be_identical<'a>( if let Some(as_type) = as_type { type2_part = as_type.get_single(); } else if inside_assertion { - let type_alias_info = codebase.type_definitions.get(name).unwrap(); - type2_part = type_alias_info.actual_type.get_single(); + if let Some(type_alias_info) = codebase.type_definitions.get(name) { + type2_part = type_alias_info.actual_type.get_single(); + } else { + break; + } } else { break; } diff --git a/tests/diff/fileWithGitMarkersIncluded/output.txt b/tests/diff/fileWithGitMarkersIncluded/output.txt index 51de6c0b..d3ceb255 100644 --- a/tests/diff/fileWithGitMarkersIncluded/output.txt +++ b/tests/diff/fileWithGitMarkersIncluded/output.txt @@ -1,2 +1,2 @@ -ERROR: InvalidHackFile - input.hack:1:1 - Invalid Hack file +ERROR: InvalidHackFile - input.hack:7:0 - A name is expected here. ERROR: NonExistentFunction - main.hack:3:5 - Function foo is not defined \ No newline at end of file diff --git a/tests/diff/fileWithGitMarkersIncludedThenNoChange/output.txt b/tests/diff/fileWithGitMarkersIncludedThenNoChange/output.txt index a4160101..88d0d4f5 100644 --- a/tests/diff/fileWithGitMarkersIncludedThenNoChange/output.txt +++ b/tests/diff/fileWithGitMarkersIncludedThenNoChange/output.txt @@ -1,2 +1,2 @@ -ERROR: InvalidHackFile - input.hack:1:1 - Invalid Hack file +ERROR: InvalidHackFile - input.hack:7:0 - A name is expected here. ERROR: NonExistentFunction - main.hack:4:5 - Function foo is not defined