From 3e56c2da9a0b36a9244da23f78d05cbbf744520d Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:01:34 -0700 Subject: [PATCH 1/3] Php-cs-fixer Increase Timeout, Replace Deprecated Properties I am becoming concerned with the increasing run-time of php-cs-fixer, especially since it can time out. Some relief may come from PR #4118, but that won't be merged for some time, if ever. So, bump up the timeout period now. Also replace properties which php-cs-fixer has deprecated with their non-deprecated equivalents. No change to any source code. --- .php-cs-fixer.dist.php | 24 +++++++---------- .../Chart/ChartsDynamicTitleTest.php | 1 + .../Reader/Xlsx/XlsxTest.php | 27 ++++++++++++------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index b0207d3857..73952e23e1 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -19,18 +19,18 @@ 'backtick_to_shell_exec' => true, 'binary_operator_spaces' => true, 'blank_line_after_namespace' => true, + 'blank_lines_before_namespace' => ['max_line_breaks' => 2, 'min_line_breaks' => 2], // we want 1 blank line before namespace 'blank_line_after_opening_tag' => true, 'blank_line_before_statement' => true, - 'braces' => true, + //'braces' => true, 'cast_spaces' => true, 'class_attributes_separation' => ['elements' => ['method' => 'one', 'property' => 'one']], // const are often grouped with other related const 'class_definition' => false, // phpcs disagree - 'class_keyword_remove' => false, // Deprecated, and ::class keyword gives us better support in IDE 'combine_consecutive_issets' => true, 'combine_consecutive_unsets' => true, 'combine_nested_dirname' => true, 'comment_to_phpdoc' => false, // interferes with annotations - 'compact_nullable_typehint' => true, + 'compact_nullable_type_declaration' => true, 'concat_space' => ['spacing' => 'one'], 'constant_case' => true, 'date_time_immutable' => false, // Break our unit tests @@ -47,7 +47,6 @@ 'encoding' => true, 'ereg_to_preg' => true, 'error_suppression' => false, // it breaks \PhpOffice\PhpSpreadsheet\Helper\Handler - 'escape_implicit_backslashes' => true, 'explicit_indirect_variable' => false, // I feel it makes the code actually harder to read 'explicit_string_variable' => false, // I feel it makes the code actually harder to read 'final_class' => false, // We need non-final classes @@ -59,7 +58,6 @@ 'fully_qualified_strict_types' => true, 'function_declaration' => true, 'function_to_constant' => true, - 'function_typehint_space' => true, 'general_phpdoc_annotation_remove' => ['annotations' => ['access', 'category', 'copyright']], 'general_phpdoc_tag_rename' => true, 'global_namespace_import' => true, @@ -93,15 +91,13 @@ 'native_constant_invocation' => false, // Micro optimization that look messy 'native_function_casing' => true, 'native_function_invocation' => false, // I suppose this would be best, but I am still unconvinced about the visual aspect of it - 'native_function_type_declaration_casing' => true, - 'new_with_braces' => true, + 'new_with_parentheses' => ['anonymous_class' => true, 'named_class' => true], 'no_alias_functions' => true, 'no_alias_language_construct_call' => true, 'no_alternative_syntax' => true, 'no_binary_string' => true, 'no_blank_lines_after_class_opening' => true, 'no_blank_lines_after_phpdoc' => true, - 'no_blank_lines_before_namespace' => false, // we want 1 blank line before namespace 'no_break_comment' => true, 'no_closing_tag' => true, 'no_empty_comment' => true, @@ -121,16 +117,14 @@ 'no_space_around_double_colon' => true, 'no_spaces_after_function_name' => true, 'no_spaces_around_offset' => true, - 'no_spaces_inside_parenthesis' => true, 'no_superfluous_elseif' => false, // Might be risky on a huge code base 'no_superfluous_phpdoc_tags' => ['allow_mixed' => true], - 'no_trailing_comma_in_list_call' => true, - 'no_trailing_comma_in_singleline_array' => true, + 'no_trailing_comma_in_singleline' => ['elements' => ['arguments', 'array_destructuring', 'array', 'group_import']], 'no_trailing_whitespace' => true, 'no_trailing_whitespace_in_comment' => true, 'no_trailing_whitespace_in_string' => false, // Too dangerous 'no_unneeded_control_parentheses' => true, - 'no_unneeded_curly_braces' => true, + 'no_unneeded_braces' => true, 'no_unneeded_final_method' => true, 'no_unreachable_default_argument_value' => true, 'no_unset_cast' => true, @@ -215,20 +209,21 @@ 'simplified_if_return' => false, // Even if technically correct we prefer to be explicit 'simplified_null_return' => false, // Even if technically correct we prefer to be explicit 'single_blank_line_at_eof' => true, - 'single_blank_line_before_namespace' => true, 'single_class_element_per_statement' => true, 'single_import_per_statement' => true, 'single_line_after_imports' => true, 'single_line_comment_style' => true, 'single_line_throw' => false, // I don't see any reason for having a special case for Exception 'single_quote' => true, - 'single_space_after_construct' => true, + //'single_space_after_construct' => true, 'single_trait_insert_per_statement' => true, 'space_after_semicolon' => true, + 'spaces_inside_parentheses' => ['space' => 'none'], 'standardize_increment' => true, 'standardize_not_equals' => true, 'static_lambda' => false, // Risky if we can't guarantee nobody use `bindTo()` 'strict_comparison' => false, // No, too dangerous to change that + 'string_implicit_backslashes' => false, // was escape_implicit_backslashes, too confusing 'strict_param' => false, // No, too dangerous to change that 'string_length_to_empty' => true, 'string_line_ending' => true, @@ -240,6 +235,7 @@ 'ternary_to_null_coalescing' => true, 'trailing_comma_in_multiline' => true, 'trim_array_spaces' => true, + 'type_declaration_spaces' => ['elements' => ['function', 'property']], // was function_typehint_space 'types_spaces' => true, 'unary_operator_spaces' => true, 'use_arrow_functions' => true, diff --git a/tests/PhpSpreadsheetTests/Chart/ChartsDynamicTitleTest.php b/tests/PhpSpreadsheetTests/Chart/ChartsDynamicTitleTest.php index 2546d60de6..7aa484d9cb 100644 --- a/tests/PhpSpreadsheetTests/Chart/ChartsDynamicTitleTest.php +++ b/tests/PhpSpreadsheetTests/Chart/ChartsDynamicTitleTest.php @@ -34,6 +34,7 @@ public function writeCharts(XlsxWriter $writer): void $writer->setIncludeCharts(true); } + #[\PHPUnit\Framework\Attributes\RunInSeparateProcess] public function testDynamicTitle(): void { // based on samples/templates/issue.3797.2007.xlsx diff --git a/tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php b/tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php index f7b86627c7..344f0666a2 100644 --- a/tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php @@ -41,6 +41,7 @@ public function testLoadXlsxRowColumnAttributes(): void } self::assertFalse($worksheet->getColumnDimension('E')->getVisible()); + $spreadsheet->disconnectWorksheets(); } public function testLoadXlsxWithStyles(): void @@ -65,6 +66,7 @@ public function testLoadXlsxWithStyles(): void ); } } + $spreadsheet->disconnectWorksheets(); } /** @@ -87,6 +89,7 @@ public function testLoadXlsxWithoutStyles(): void $reloadedWorksheet = $reloadedSpreadsheet->getActiveSheet(); self::assertEquals('TipoDato', $reloadedWorksheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } /** @@ -108,6 +111,7 @@ public function testLoadXlsxWithEmptyStyles(): void $reloadedWorksheet = $reloadedSpreadsheet->getActiveSheet(); self::assertEquals('TipoDato', $reloadedWorksheet->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } public function testLoadXlsxAutofilter(): void @@ -125,6 +129,7 @@ public function testLoadXlsxAutofilter(): void AutoFilter\Column::AUTOFILTER_FILTERTYPE_FILTER, $autofilter->getColumn('A')->getFilterType() ); + $spreadsheet->disconnectWorksheets(); } public function testLoadXlsxPageSetup(): void @@ -163,18 +168,19 @@ public function testLoadXlsxConditionalFormatting(): void self::assertEquals(Conditional::OPERATOR_BETWEEN, $conditionalRule->getOperatorType()); self::assertEquals(['200', '400'], $conditionalRule->getConditions()); self::assertInstanceOf(Style::class, $conditionalRule->getStyle()); + $spreadsheet->disconnectWorksheets(); } /** * Test load Xlsx file without cell reference. - * - * @doesNotPerformAssertions */ public function testLoadXlsxWithoutCellReference(): void { $filename = 'tests/data/Reader/XLSX/without_cell_reference.xlsx'; $reader = new Xlsx(); - $reader->load($filename); + $spreadsheet = $reader->load($filename); + self::assertSame(1, $spreadsheet->getActiveSheet()->getCell('A1')->getValue()); + $spreadsheet->disconnectWorksheets(); } /** @@ -185,24 +191,26 @@ public function testLoadWithReadFilter(): void $filename = 'tests/data/Reader/XLSX/without_cell_reference.xlsx'; $reader = new Xlsx(); $reader->setReadFilter(new OddColumnReadFilter()); - $data = $reader->load($filename)->getActiveSheet()->toArray(); + $spreadsheet = $reader->load($filename); + $data = $spreadsheet->getActiveSheet()->toArray(); $ref = [1.0, null, 3.0, null, 5.0, null, 7.0, null, 9.0, null]; for ($i = 0; $i < 10; ++$i) { self::assertEquals($ref, \array_slice($data[$i], 0, 10, true)); } + $spreadsheet->disconnectWorksheets(); } /** * Test load Xlsx file with drawing having double attributes. - * - * @doesNotPerformAssertions */ public function testLoadXlsxWithDoubleAttrDrawing(): void { $filename = 'tests/data/Reader/XLSX/double_attr_drawing.xlsx'; $reader = new Xlsx(); - $reader->load($filename); + $spreadsheet = $reader->load($filename); + self::assertSame('TOSHIBA_HITACHI_SKYWORTH', $spreadsheet->getActiveSheet()->getTitle()); + $spreadsheet->disconnectWorksheets(); } /** @@ -219,8 +227,8 @@ public function testLoadSaveWithEmptyDrawings(): void $writer->save($resultFilename); $excel = $reader->load($resultFilename); unlink($resultFilename); - // Fake assert. The only thing we need is to ensure the file is loaded without exception - self::assertNotNull($excel); + self::assertSame(1.0, $excel->getActiveSheet()->getCell('A1')->getValue()); + $excel->disconnectWorksheets(); } /** @@ -271,5 +279,6 @@ public function testLoadDataOnlyLoadsAlsoTables(): void ['e', 'f'], ['g', 'h'], ], $secondSheet->toArray()); + $excel->disconnectWorksheets(); } } From 9bc51a18c192662ab9cb9fac4577e9e7514c09c1 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:06:06 -0700 Subject: [PATCH 2/3] Forgot One Change --- .php-cs-fixer.dist.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 73952e23e1..3dd86ce177 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -10,7 +10,7 @@ $config ->setRiskyAllowed(true) ->setFinder($finder) - ->setParallelConfig(PhpCsFixer\Runner\Parallel\ParallelConfigFactory::detect()) + ->setParallelConfig(PhpCsFixer\Runner\Parallel\ParallelConfigFactory::detect(null, 600)) ->setCacheFile(sys_get_temp_dir() . '/php-cs-fixer' . preg_replace('~\W~', '-', __DIR__)) ->setRules([ 'align_multiline_comment' => true, From b1d43ee3a3c832608bda2188bf97bc75e31e84c3 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:09:37 -0700 Subject: [PATCH 3/3] Remove Some Commented Out Lines --- .php-cs-fixer.dist.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 3dd86ce177..c4711f12a0 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -22,7 +22,6 @@ 'blank_lines_before_namespace' => ['max_line_breaks' => 2, 'min_line_breaks' => 2], // we want 1 blank line before namespace 'blank_line_after_opening_tag' => true, 'blank_line_before_statement' => true, - //'braces' => true, 'cast_spaces' => true, 'class_attributes_separation' => ['elements' => ['method' => 'one', 'property' => 'one']], // const are often grouped with other related const 'class_definition' => false, // phpcs disagree @@ -169,7 +168,6 @@ 'phpdoc_align' => false, // Waste of time 'phpdoc_annotation_without_dot' => true, 'phpdoc_indent' => true, - //'phpdoc_inline_tag' => true, 'phpdoc_line_span' => false, // Unfortunately our old comments turn even uglier with this 'phpdoc_no_access' => true, 'phpdoc_no_alias_tag' => true, @@ -215,7 +213,6 @@ 'single_line_comment_style' => true, 'single_line_throw' => false, // I don't see any reason for having a special case for Exception 'single_quote' => true, - //'single_space_after_construct' => true, 'single_trait_insert_per_statement' => true, 'space_after_semicolon' => true, 'spaces_inside_parentheses' => ['space' => 'none'],