diff options
| -rw-r--r-- | crates/shirabe-class-map-generator/src/php_file_parser.rs | 78 | ||||
| -rw-r--r-- | crates/shirabe-php-shim/src/runtime.rs | 5 | ||||
| -rw-r--r-- | crates/shirabe-php-shim/src/string.rs | 12 | ||||
| -rw-r--r-- | crates/shirabe/src/util/filesystem.rs | 66 |
4 files changed, 62 insertions, 99 deletions
diff --git a/crates/shirabe-class-map-generator/src/php_file_parser.rs b/crates/shirabe-class-map-generator/src/php_file_parser.rs index 88676407..712fb4ab 100644 --- a/crates/shirabe-class-map-generator/src/php_file_parser.rs +++ b/crates/shirabe-class-map-generator/src/php_file_parser.rs @@ -4,9 +4,9 @@ use crate::php_file_cleaner::PhpFileCleaner; use indexmap::IndexMap; use shirabe_external_packages::composer::pcre::{CaptureKey, Preg}; use shirabe_php_shim::{ - CmpOp, HHVM_VERSION, PHP_EOL, PHP_VERSION_ID, RuntimeException, error_get_last, file_exists, - file_get_contents, function_exists, is_file, is_readable, ltrim, php_strip_whitespace, - str_replace_array, strrpos, substr, trim, version_compare, + CmpOp, HHVM_VERSION, PHP_EOL, PHP_VERSION_ID, RuntimeException, file_exists, file_get_contents, + function_exists, is_file, is_readable, ltrim, php_strip_whitespace, str_replace_array, strrpos, + substr, trim, version_compare, }; use std::sync::OnceLock; @@ -21,46 +21,42 @@ impl PhpFileParser { } // Use @ here instead of Silencer to actively suppress 'unhelpful' output - let contents = php_strip_whitespace(path); - if contents.is_empty() { - let message: String; - if !file_exists(path) { - message = format!( - "File at \"{}\" does not exist, check your classmap definitions", - path - ); - } else if !Self::is_readable(path) { - message = format!( - "File at \"{}\" is not readable, check its permissions", - path - ); - } else if trim(file_get_contents(path).unwrap_or_default().as_str(), None).is_empty() { - // The input file was really empty and thus contains no classes - return Ok(vec![]); - } else { - message = format!( - "File at \"{}\" could not be parsed as PHP, it may be binary or corrupted", - path - ); - } + let contents = match php_strip_whitespace(path) { + Ok(contents) if !contents.is_empty() => contents, + stripped => { + let mut message: String; + if !file_exists(path) { + message = format!( + "File at \"{}\" does not exist, check your classmap definitions", + path + ); + } else if !Self::is_readable(path) { + message = format!( + "File at \"{}\" is not readable, check its permissions", + path + ); + } else if trim(file_get_contents(path).unwrap_or_default().as_str(), None) + .is_empty() + { + // The input file was really empty and thus contains no classes + return Ok(vec![]); + } else { + message = format!( + "File at \"{}\" could not be parsed as PHP, it may be binary or corrupted", + path + ); + } - let error = error_get_last(); - let mut message = message; - if let Some(error) = error - && let Some(err_msg) = error.get("message") - { - message = format!( - "{}{}{}{}{}", - message, - PHP_EOL, - "The following message may be helpful:", - PHP_EOL, - err_msg.as_string().unwrap_or("") - ); - } + if let Err(error) = stripped { + message = format!( + "{}{}{}{}{}", + message, PHP_EOL, "The following message may be helpful:", PHP_EOL, error + ); + } - return Err(RuntimeException::new(message).into()); - } + return Err(RuntimeException::new(message).into()); + } + }; // return early if there is no chance of matching anything in this file let pattern = format!("{{\\b(?:class|interface|trait{})\\s}}i", extra_types); diff --git a/crates/shirabe-php-shim/src/runtime.rs b/crates/shirabe-php-shim/src/runtime.rs index 2f8b79f1..6b165567 100644 --- a/crates/shirabe-php-shim/src/runtime.rs +++ b/crates/shirabe-php-shim/src/runtime.rs @@ -360,11 +360,6 @@ pub fn call_php_callable(_callback: &PhpMixed, _args: &[PhpMixed]) -> PhpMixed { todo!() } -// The shim does not raise PHP-level errors, so there is never a last error. -pub fn error_get_last() -> Option<IndexMap<String, PhpMixed>> { - None -} - pub fn ini_set(_varname: &str, _value: &str) -> Option<String> { // TODO(php-runtime): ini_set must return the previous value and have its override observed by a // subsequent ini_get; ini_get is currently a static lookup, so overrides cannot be wired up yet. diff --git a/crates/shirabe-php-shim/src/string.rs b/crates/shirabe-php-shim/src/string.rs index 575bc16d..78688eed 100644 --- a/crates/shirabe-php-shim/src/string.rs +++ b/crates/shirabe-php-shim/src/string.rs @@ -846,17 +846,15 @@ pub fn ucfirst(s: &str) -> String { } } -pub fn php_strip_whitespace(path: impl AsRef<std::path::Path>) -> String { +pub fn php_strip_whitespace(path: impl AsRef<std::path::Path>) -> Result<String, std::io::Error> { // PHP `php_strip_whitespace()` tokenizes the source and re-emits it with comments removed and // each run of whitespace collapsed to a single space. There is no PHP tokenizer in the shim, so // this is a hand-written lexer that reproduces the observable effect for the cases the class-map // generator depends on: it preserves single-quoted, double-quoted, backtick and heredoc/nowdoc // string contents verbatim while dropping `//`, `#` and `/* */` comments and squeezing - // whitespace. On any read failure it returns an empty string, mirroring `@php_strip_whitespace`. - let contents = match std::fs::read(path.as_ref()) { - Ok(bytes) => bytes, - Err(_) => return String::new(), - }; + // whitespace. PHP returns an empty string on a read failure and leaves the reason in the warning + // text; this returns the io::Error instead so callers can report it. + let contents = std::fs::read(path.as_ref())?; let b = contents; let n = b.len(); @@ -981,7 +979,7 @@ pub fn php_strip_whitespace(path: impl AsRef<std::path::Path>) -> String { i += 1; } - String::from_utf8_lossy(&out).into_owned() + Ok(String::from_utf8_lossy(&out).into_owned()) } pub fn hexdec(_s: &str) -> i64 { diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs index 925cb38a..9c4f4dbf 100644 --- a/crates/shirabe/src/util/filesystem.rs +++ b/crates/shirabe/src/util/filesystem.rs @@ -8,9 +8,9 @@ use shirabe_external_packages::symfony::filesystem::exception::IOException; use shirabe_external_packages::symfony::finder::Finder; use shirabe_php_shim::{ ErrorException, LogicException, PhpMixed, RuntimeException, array_pop, basename, chdir, - clearstatcache, clearstatcache2, copy, dirname, error_get_last, explode, fclose, feof, - file_exists, file_get_contents, file_put_contents, fileatime, filemtime, filesize, fopen, - fread, function_exists, fwrite, implode, is_dir, is_file, is_link, is_readable, lstat, mkdir, + clearstatcache, clearstatcache2, copy, dirname, explode, fclose, feof, file_exists, + file_get_contents, file_put_contents, fileatime, filemtime, filesize, fopen, fread, + function_exists, fwrite, implode, is_dir, is_file, is_link, is_readable, lstat, mkdir, php_regex, rename, rmdir, rtrim, str_contains, str_repeat, str_replace, str_starts_with, strlen, strpos, strtoupper, strtr, substr, substr_count, symlink, touch, unlink, usleep, var_export, @@ -315,28 +315,20 @@ impl Filesystem { .into()); } - if is_link(directory) && !self.unlink_implementation(Path::new(directory)) { + if is_link(directory) + && let Err(last_error) = self.unlink_implementation(Path::new(directory)) + { return Err(RuntimeException::new(format!( "Could not delete symbolic link {}: {}", - directory, - error_get_last() - .as_ref() - .and_then(|m| m.get("message")) - .and_then(|v| v.as_string()) - .unwrap_or("") + directory, last_error )) .into()); } - if mkdir(directory, 0o777, true).is_err() { + if let Err(last_error) = mkdir(directory, 0o777, true) { let e = RuntimeException::new(format!( "{} does not exist and could not be created: {}", - directory, - error_get_last() - .as_ref() - .and_then(|m| m.get("message")) - .and_then(|v| v.as_string()) - .unwrap_or("") + directory, last_error )); // in pathological cases with paths like path/to/broken-symlink/../foo is_dir will fail to detect path/to/foo @@ -361,24 +353,15 @@ impl Filesystem { pub fn unlink(&self, path: impl AsRef<Path>) -> anyhow::Result<bool> { let path = path.as_ref(); let mut unlinked = self.unlink_implementation(path); - if !unlinked { + if unlinked.is_err() { // retry after a bit on windows since it tends to be touchy with mass removals if Platform::is_windows() { usleep(350000); unlinked = self.unlink_implementation(path); } - if !unlinked { - let error = error_get_last(); - let mut message = format!( - "Could not delete {}: {}", - path.display(), - error - .as_ref() - .and_then(|m| m.get("message")) - .and_then(|v| v.as_string()) - .unwrap_or("") - ); + if let Err(last_error) = unlinked { + let mut message = format!("Could not delete {}: {}", path.display(), last_error); if Platform::is_windows() { message.push_str("\nThis can be due to an antivirus or the Windows Search Indexer locking the file while they are analyzed"); } @@ -393,25 +376,16 @@ impl Filesystem { /// Attempts to rmdir a file and in case of failure retries after 350ms on windows pub fn rmdir(&self, path: impl AsRef<Path>) -> anyhow::Result<bool> { let path = path.as_ref(); - let mut deleted = rmdir(path).is_ok(); - if !deleted { + let mut deleted = rmdir(path); + if deleted.is_err() { // retry after a bit on windows since it tends to be touchy with mass removals if Platform::is_windows() { usleep(350000); - deleted = rmdir(path).is_ok(); + deleted = rmdir(path); } - if !deleted { - let error = error_get_last(); - let mut message = format!( - "Could not delete {}: {}", - path.display(), - error - .as_ref() - .and_then(|m| m.get("message")) - .and_then(|v| v.as_string()) - .unwrap_or("") - ); + if let Err(last_error) = deleted { + let mut message = format!("Could not delete {}: {}", path.display(), last_error); if Platform::is_windows() { message.push_str("\nThis can be due to an antivirus or the Windows Search Indexer locking the file while they are analyzed"); } @@ -920,12 +894,12 @@ impl Filesystem { /// delete symbolic link implementation (commonly known as "unlink()") /// /// symbolic links on windows which link to directories need rmdir instead of unlink - fn unlink_implementation(&self, path: &Path) -> bool { + fn unlink_implementation(&self, path: &Path) -> Result<(), std::io::Error> { if Platform::is_windows() && is_dir(path) && is_link(path) { - return rmdir(path).is_ok(); + return rmdir(path); } - unlink(path).is_ok() + unlink(path) } /// Creates a relative symlink from $link to $target |