Task #70584

Reduce thrown E_NOTICEs

Added by Oliver Eglseder about 4 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
Code Cleanup
Start date:
2018-03-07
Due date:
% Done:

0%

TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Sprint Focus:
On Location Sprint

Description

Every time PHP tries to access an index of an array, which is not set, or a variable that is not defined PHP generated an E_NOTICE and uses NULL as resulting value.

This is bad because of three major reasons:
1. Bugs resulting from an invalid evaluation of the value are very hard to find.
2. You can not trust the type of the variable so you have to check it anywhere you use it.
3. Not checking keys is considered bad code.

And a lot of other (major and minor) reasons to check the key/variable before accessing it, including tiny performance improvements as a positive side effect.

Mission:

Always check if the variable/key exists before accessing it.
If the value is not existent use the expected default value (e.g.: empty string, zero, null..)

How to work on this:

Edit UnitTests.xml in core/Build and set convertNoticesToExceptions="true" and stopOnFailure="true" as <phpunit> tag attributes.
Go to \TYPO3\CMS\Core\Core\SystemEnvironmentBuilder::initializeBasicErrorReporting and set error_reporting(E_ALL)
Run all unit tests. PHPUnit will halt on the first error.

Create a child issue on this ticket that combines some changes into logic units.
There shouldn't be more than 20 changes per patch to keep them simple and increase velocity. Smaller patches are getting approved and merged faster.


Related issues

Related to TYPO3 Core - Task #70587: Remove the first few E_NOTICEs occuring in the unit tests Closed 2015-10-11
Related to TYPO3 Core - Task #70590: Remove all notices thrown in BackendUtilityTest Closed 2015-10-11
Related to TYPO3 Core - Task #71236: Provide correct dummy data for DatabaseLanguageRowsTest Closed 2015-11-01
Related to TYPO3 Core - Bug #71292: Reduce E_NOTICEs by providing correct test values and issets Closed 2015-11-03
Related to TYPO3 Core - Bug #84156: avoid E_NOTICE reporting in ArrayUtility Closed 2018-03-07
Related to TYPO3 Core - Bug #84161: avoid E_NOTICE reporting in ArrayUtility - reloaded Closed 2018-03-07
Related to TYPO3 Core - Epic #84280: Make unit tests notice free Closed 2018-03-15

Associated revisions

Revision 57055c98 (diff)
Added by Oliver Eglseder about 4 years ago

[TASK] Remove the first few E_NOTICEs occuring in the unit tests

PHP throws E_NOTICE when not existing array keys are accessed.
The received value will become null.

This patch aims to reduce the number of generated notices,
to increase readability and reliability of the changed methods.

Resolves: #70587
Related: #70584
Releases: master
Change-Id: I23ee39b0d89f1be828565cbe6c745b6eeefdcfde
Reviewed-on: https://review.typo3.org/43988
Reviewed-by: Markus Klein <>
Tested-by: Markus Klein <>
Reviewed-by: Benni Mack <>
Tested-by: Benni Mack <>

Revision f78b2b28 (diff)
Added by Oliver Eglseder about 4 years ago

[TASK] Provide correct data for DatabaseLanguageRowsTest

The dummy data provided in DatabaseLanguageRowsTest
leads to some E_NOTICEs.

This patch aims to provide the minimal correct set of dummy
data needed to run the affected tests without any E_NOTICE

Resolves: #71236
Related: #70584
Releases: master
Change-Id: I159f71325bdd26482ac1bdfb8176ff707acbc036
Reviewed-on: https://review.typo3.org/44470
Reviewed-by: Wouter Wolters <>
Tested-by: Wouter Wolters <>
Reviewed-by: Morton Jonuschat <>
Tested-by: Morton Jonuschat <>

Revision 4a74f00e (diff)
Added by Pawel Cieslik over 1 year ago

[TASK] E_NOTICE reduction

Reduce number of E_NOTICE thrown by TYPO3

Resolves: #70584
Releases: master
Change-Id: I46fb54e51b24af5721efaa9507b32b86f38fc325
Reviewed-on: https://review.typo3.org/54884
Reviewed-by: Jan Helke <>
Tested-by: Jan Helke <>
Tested-by: TYPO3com <>
Reviewed-by: Christian Kuhn <>
Tested-by: Christian Kuhn <>

History

#1 Updated by Riccardo De Contardi over 3 years ago

  • Category set to Code Cleanup

#2 Updated by Riccardo De Contardi over 2 years ago

  • Target version changed from 8 LTS to Candidate for Major Version

#3 Updated by Gerrit Code Review almost 2 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#4 Updated by Gerrit Code Review almost 2 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#5 Updated by Gerrit Code Review almost 2 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#6 Updated by Riccardo De Contardi over 1 year ago

  • Related to Bug #84156: avoid E_NOTICE reporting in ArrayUtility added

#7 Updated by Ralf Zimmermann over 1 year ago

  • Related to Bug #84161: avoid E_NOTICE reporting in ArrayUtility - reloaded added

#8 Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#9 Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#10 Updated by Łukasz Uznański over 1 year ago

  • Sprint Focus set to On Location Sprint

#11 Updated by Christian Kuhn over 1 year ago

  • Related to Epic #84280: Make unit tests notice free added

#12 Updated by Gerrit Code Review over 1 year ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/54884

#13 Updated by Jan Helke over 1 year ago

  • Status changed from Under Review to Resolved
  • Assignee deleted (Oliver Eglseder)

All issues connected to this task are either closed or resolved. I will close the ticket and we will handle E_NOTICEs further on in https://forge.typo3.org/issues/84280

#14 Updated by Benni Mack about 1 year ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF