Merge pull request #11032 from creative-commoners/pulls/5.1/remove-todo

MNT Remove TODO comments

sake

@@ -90,7 +90,6 @@ if [ "$1" = "-start" ]; then
 			url=$3
 		fi
 
-		# TODO: Give a globally unique processname by including the projectname as well
 		processname=$2
 
 		daemon -n $processname -r -D $base --pidfile=$pidfile --stdout=$outlog --stderr=$errlog $sake $url
@@ -105,7 +104,6 @@ if [ "$1" = "-stop" ]; then
 	if [ -f $pidfile ]; then
 		echo "Stopping service $2"
 
-		# TODO: This is a bad way of killing the process
 		kill -KILL `cat $pidfile`
 		unlink $pidfile
 	else

src/Control/ContentNegotiator.php

@@ -25,10 +25,6 @@ use SilverStripe\Core\Injector\Injectable;
  *
  * Please see http://webkit.org/blog/68/understanding-html-xml-and-xhtml/ for further information.
  *
- * @todo Check for correct XHTML doctype in xhtml()
- * @todo Allow for other HTML4 doctypes (e.g. Transitional) in html()
- * @todo Make content replacement and doctype setting two separately configurable behaviours
- *
  * Some developers might know what they're doing and don't want ContentNegotiator messing with their
  * HTML4 doctypes, but still find it useful to have self-closing tags removed.
  */
@@ -169,8 +165,6 @@ class ContentNegotiator
      * <input>, checked, selected).
      *
      * @param HTTPResponse $response
-     *
-     * @todo Search for more xhtml replacement
      */
     public function xhtml(HTTPResponse $response)
     {

src/Control/Controller.php

@@ -146,8 +146,6 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
     /**
      * A bootstrap for the handleRequest method
      *
-     * @todo setDataModel and setRequest are redundantly called in parent::handleRequest() - sort this out
-     *
      * @param HTTPRequest $request
      */
     protected function beforeHandleRequest(HTTPRequest $request)

src/Control/CookieJar.php

@@ -11,10 +11,6 @@ use LogicException;
  * This backend allows one to better test Cookie setting and separate cookie
  * handling from the core
  *
- * @todo Create a config array for defaults (eg: httpOnly, secure, path, domain, expiry)
- * @todo A getter for cookies that haven't been sent to the browser yet
- * @todo Tests / a way to set the state without hacking with $_COOKIE
- * @todo Store the meta information around cookie setting (path, domain, secure, etc)
  */
 class CookieJar implements Cookie_Backend
 {

src/Control/HTTP.php

@@ -121,7 +121,6 @@ class HTTP
             $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/i";
         }
         // Replace css styles
-        // @todo - http://www.css3.info/preview/multiple-backgrounds/
         $styles = ['background-image', 'background', 'list-style-image', 'list-style', 'content'];
         foreach ($styles as $style) {
             $regExps[] = "/($style:[^;]*url *\\(\")([^\"]+)(\"\\))/i";

src/Control/Middleware/HTTPCacheControlMiddleware.php

@@ -28,8 +28,6 @@ class HTTPCacheControlMiddleware implements HTTPMiddleware, Resettable
     /**
      * Generate response for the given request
      *
-     * @todo Refactor HTTP::add_cache_headers() (e.g. etag handling) into this middleware
-     *
      * @param HTTPRequest $request
      * @param callable $delegate
      * @return HTTPResponse

src/Control/RSS/RSSFeed.php

@@ -18,7 +18,6 @@ use SilverStripe\View\ViewableData;
  * RSSFeed class
  *
  * This class is used to create an RSS feed.
- * @todo Improve documentation
  */
 class RSSFeed extends ViewableData
 {
@@ -215,8 +214,6 @@ class RSSFeed extends ViewableData
     /**
      * Output the feed to the browser.
      *
-     * TODO: Pass $response object to ->outputToBrowser() to loosen dependence on global state for easier testing/prototyping so dev can inject custom HTTPResponse instance.
-     *
      * @return DBHTMLText
      */
     public function outputToBrowser()

src/Control/RequestHandler.php

@@ -171,8 +171,6 @@ class RequestHandler extends ViewableData
         // We used to put "handleAction" as the action on controllers, but (a) this could only be called when
         // you had $Action in your rule, and (b) RequestHandler didn't have one. $Action is better
         if ($action == 'handleAction') {
-            // TODO Fix LeftAndMain usage
-            // Deprecation::notice('3.2.0', 'Calling handleAction directly is deprecated - use $Action instead');
             $action = '$Action';
         }
 

src/Control/SimpleResourceURLGenerator.php

@@ -140,7 +140,6 @@ class SimpleResourceURLGenerator implements ResourceURLGenerator
             // All resources mapped directly to _resources/
             $relativePath = Path::join(RESOURCES_DIR, $relativePath);
         } elseif (stripos($relativePath ?? '', ManifestFileFinder::VENDOR_DIR . DIRECTORY_SEPARATOR) === 0) {
-            // @todo Non-public dir support will be removed in 5.0, so remove this block there
             // If there is no public folder, map to _resources/ but trim leading vendor/ too (4.0 compat)
             $relativePath = Path::join(
                 RESOURCES_DIR,

src/Core/BaseKernel.php

@@ -114,7 +114,6 @@ abstract class BaseKernel implements Kernel
         $this->setModuleLoader($moduleLoader);
 
         // Config loader
-        // @todo refactor CoreConfigFactory
         $configFactory = new CoreConfigFactory($manifestCacheFactory);
         $configManifest = $configFactory->createRoot();
         $configLoader = ConfigLoader::inst();

src/Core/ClassInfo.php

@@ -82,8 +82,6 @@ class ClassInfo
     }
 
     /**
-     * @todo Move this to SS_Database or DB
-     *
      * @param string $tableName
      * @return bool
      */
@@ -130,8 +128,6 @@ class ClassInfo
      * Returns an array of the current class and all its ancestors and children
      * which require a DB table.
      *
-     * @todo Move this into {@see DataObjectSchema}
-     *
      * @param string|object $nameOrObject Class or object instance
      * @return array
      */

src/Core/Convert.php

@@ -204,7 +204,6 @@ class Convert
      * Warning: Does not decode array keys
      *
      * @uses html2raw()
-     * @todo Currently &#xxx; entries are stripped; they should be converted
      * @param mixed $val
      * @return array|string
      */

src/Core/Extensible.php

@@ -217,8 +217,6 @@ trait Extensible
      * Clears any previously created singletons through {@link singleton()}
      * to avoid side-effects from stale extension information.
      *
-     * @todo Add support for removing extensions with parameters
-     *
      * @param string $extension class name of an {@link Extension} subclass, without parameters
      */
     public static function remove_extension($extension)

src/Core/Injector/Injector.php

@@ -666,10 +666,6 @@ class Injector implements ContainerInterface
     /**
      * Inject $object with available objects from the service cache
      *
-     * @todo Track all the existing objects that have had a service bound
-     * into them, so we can update that binding at a later point if needbe (ie
-     * if the managed service changes)
-     *
      * @param object $object
      *              The object to inject
      * @param string $asType

src/Dev/BulkLoader.php

@@ -12,10 +12,6 @@ use SilverStripe\View\ViewableData;
  *
  * You can configure column-handling,
  *
- * @todo Add support for adding/editing has_many relations.
- * @todo Add support for deep chaining of relation properties (e.g. Player.Team.Stats.GoalCount)
- * @todo Character conversion
- *
  * @see http://tools.ietf.org/html/rfc4180
  * @author Ingo Schommer, Silverstripe Ltd. (<firstname>@silverstripe.com)
  */
@@ -217,7 +213,6 @@ abstract class BulkLoader extends ViewableData
      * ]
      * </code>
      *
-     * @todo Mix in custom column mappings
      *
      * @return array
      **/

src/Dev/BulkLoader_Result.php

@@ -11,8 +11,6 @@ use SilverStripe\View\ArrayData;
  * Encapsulates the result of a {@link BulkLoader} import
  * (usually through the {@link BulkLoader->processAll()} method).
  *
- * @todo Refactor to support lazy-loaded DataObjectSets once they are implemented.
- *
  * @author Ingo Schommer, Silverstripe Ltd. (<firstname>@silverstripe.com)
  */
 class BulkLoader_Result implements \Countable

src/Dev/CliDebugView.php

@@ -9,8 +9,6 @@ use SilverStripe\Core\Convert;
 /**
  * A basic HTML wrapper for stylish rendering of a development info view.
  * Used to output error messages, and test results.
- *
- * @todo Perhaps DebugView should be an interface / ABC, implemented by HTMLDebugView and CliDebugView?
  */
 class CliDebugView extends DebugView
 {

src/Dev/Constraint/ViewableDataContains.php

@@ -74,8 +74,6 @@ class ViewableDataContains extends Constraint implements TestOnly
 
     /**
      * Returns a string representation of the object.
-     *
-     * @todo: add representation for more than one match
      */
     public function toString(): string
     {

src/Dev/CsvBulkLoader.php

@@ -15,9 +15,6 @@ use SilverStripe\ORM\DataObject;
  * input.
  *
  * @see http://tools.ietf.org/html/rfc4180
- *
- * @todo Support for deleting existing records not matched in the import
- * (through relation checks)
  */
 class CsvBulkLoader extends BulkLoader
 {
@@ -160,9 +157,6 @@ class CsvBulkLoader extends BulkLoader
     }
 
     /**
-     * @todo Better messages for relation checks and duplicate detection
-     * Note that columnMap isn't used.
-     *
      * @param array $record
      * @param array $columnMap
      * @param BulkLoader_Result $results
@@ -261,7 +255,6 @@ class CsvBulkLoader extends BulkLoader
             $obj->write();
         }
 
-        // @todo better message support
         $message = '';
 
         // save to results
@@ -289,8 +282,6 @@ class CsvBulkLoader extends BulkLoader
      * Find an existing objects based on one or more uniqueness columns
      * specified via {@link self::$duplicateChecks}.
      *
-     * @todo support $columnMap
-     *
      * @param array $record CSV data column
      * @param array $columnMap
      * @return DataObject

src/Dev/Debug.php

@@ -25,9 +25,6 @@ use SilverStripe\Security\Security;
  * Errors handled by this class are passed along to {@link SS_Log}.
  * For configuration information, see the {@link SS_Log}
  * class documentation.
- *
- * @todo add support for user defined config: Debug::die_on_notice(true | false)
- * @todo better way of figuring out the error context to display in highlighted source
  */
 class Debug
 {
@@ -212,7 +209,6 @@ class Debug
             // being called again.
             // This basically calls Permission::checkMember($_SESSION['loggedInAs'], 'ADMIN');
 
-            // @TODO - Rewrite safely using DataList::filter
             $memberID = $_SESSION['loggedInAs'];
             $permission = DB::prepared_query(
                 '

src/Dev/DevelopmentAdmin.php

@@ -19,11 +19,6 @@ use Exception;
  *
  * Configured in framework/_config/dev.yml, with the config key registeredControllers being
  * used to generate the list of links for /dev.
- *
- * @todo documentation for how to add new unit tests and tasks
- * @todo do we need buildDefaults and generatesecuretoken? if so, register in the list
- * @todo cleanup errors() it's not even an allowed action, so can go
- * @todo cleanup index() html building
  */
 class DevelopmentAdmin extends Controller
 {

src/Dev/State/ExtensionTestState.php

@@ -104,9 +104,6 @@ class ExtensionTestState implements TestState
 
     public function tearDownOnce($class)
     {
-        // @todo: This isn't strictly necessary to restore extensions, but only to ensure that
-        // Object::$extra_methods is properly flushed. This should be replaced with a simple
-        // flush mechanism for each $class.
         /** @var string|DataObject $dataClass */
 
         // Remove extensions added for testing

src/Forms/CheckboxSetField.php

@@ -36,11 +36,6 @@ use SilverStripe\View\ArrayData;
  *   the database records.
  * - If the field name matches a database field, a comma-separated list of values will be saved to that field.  The
  *   keys can be text or numbers.
- *
- * @todo Document the different source data that can be used
- * with this form field - e.g ComponentSet, ArrayList,
- * array. Is it also appropriate to accept so many different
- * types of data when just using an array would be appropriate?
  */
 class CheckboxSetField extends MultiSelectField
 {
@@ -48,9 +43,6 @@ class CheckboxSetField extends MultiSelectField
     protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_MULTISELECT;
 
     /**
-     * @todo Explain different source data that can be used with this field,
-     * e.g. SQLMap, ArrayList or an array.
-     *
      * @param array $properties
      * @return DBHTMLText
      */

src/Forms/CompositeField.php

@@ -121,8 +121,6 @@ class CompositeField extends FormField
      * If the CompositeField doesn't have a name, but we still want the ID/name to be set.
      * This code generates the ID from the nested children.
      *
-     * @todo this is temporary, and should be removed when FormTemplateHelper is updated to handle ID for CompositeFields with no name
-     *
      * @return String $name
      */
     public function getName()

src/Forms/CurrencyField.php

@@ -9,8 +9,6 @@ use SilverStripe\ORM\FieldType\DBCurrency;
  * Limited to US-centric formats, including a hardcoded currency
  * symbol and decimal separators.
  * See {@link MoneyField} for a more flexible implementation.
- *
- * @todo Add localization support, see http://open.silverstripe.com/ticket/2931
  */
 class CurrencyField extends TextField
 {

src/Forms/DefaultFormFactory.php

@@ -67,7 +67,6 @@ class DefaultFormFactory implements FormFactory
     protected function getFormFields(RequestHandler $controller = null, $name, $context = [])
     {
         // Fall back to standard "getCMSFields" which itself uses the FormScaffolder as a fallback
-        // @todo Deprecate or formalise support for getCMSFields()
         $fields = $context['Record']->getCMSFields();
         $this->invokeWithExtensions('updateFormFields', $fields, $controller, $name, $context);
         return $fields;
@@ -83,7 +82,6 @@ class DefaultFormFactory implements FormFactory
      */
     protected function getFormActions(RequestHandler $controller = null, $name, $context = [])
     {
-        // @todo Deprecate or formalise support for getCMSActions()
         $actions = $context['Record']->getCMSActions();
         $this->invokeWithExtensions('updateFormActions', $actions, $controller, $name, $context);
         return $actions;

src/Forms/FieldGroup.php

@@ -105,9 +105,6 @@ class FieldGroup extends CompositeField
      * Returns the name (ID) for the element.
      * In some cases the FieldGroup doesn't have a title, but we still want
      * the ID / name to be set. This code, generates the ID from the nested children
-     *
-     * TODO this is temporary, and should be removed when FormTemplateHelper is updated to handle ID
-     *  for CompositeFields with no name
      */
     public function getName()
     {

src/Forms/FieldList.php

@@ -411,8 +411,6 @@ class FieldList extends ArrayList
     /**
      * Returns the specified tab object, creating it if necessary.
      *
-     * @todo Support recursive creation of TabSets
-     *
      * @param string $tabName The tab to return, in the form "Tab.Subtab.Subsubtab".
      *   Caution: Does not recursively create TabSet instances, you need to make sure everything
      *   up until the last tab in the chain exists.
@@ -462,8 +460,6 @@ class FieldList extends ArrayList
      * Returns a named field.
      * You can use dot syntax to get fields from child composite fields
      *
-     * @todo Implement similarly to dataFieldByName() to support nested sets - or merge with dataFields()
-     *
      * @param string $name
      * @return FormField|null
      */

src/Forms/FormField.php

@@ -123,8 +123,6 @@ class FormField extends RequestHandler
      * Adds a title attribute to the markup.
      *
      * @var string
-     *
-     * @todo Implement in all subclasses
      */
     protected $description;
 
@@ -1425,8 +1423,6 @@ class FormField extends RequestHandler
      *
      * @param array $schemaData - The data to be merged with $this->schemaData.
      * @return FormField
-     *
-     * @todo Add deep merging of arrays like `data` and `attributes`.
      */
     public function setSchemaData($schemaData = [])
     {
@@ -1447,8 +1443,6 @@ class FormField extends RequestHandler
     }
 
     /**
-     * @todo Throw exception if value is missing, once a form field schema is mandatory across the CMS
-     *
      * @return string
      */
     public function getSchemaDataType()
@@ -1501,8 +1495,6 @@ class FormField extends RequestHandler
      *
      * @param array $schemaState The data to be merged with $this->schemaData.
      * @return FormField
-     *
-     * @todo Add deep merging of arrays like `data` and `attributes`.
      */
     public function setSchemaState($schemaState = [])
     {
@@ -1529,7 +1521,6 @@ class FormField extends RequestHandler
      * Includes validation data if the field is associated to a {@link Form},
      * and {@link Form->validate()} has been called.
      *
-     * @todo Make form / field messages not always stored as html; Store value / casting as separate values.
      * @return array
      */
     public function getSchemaStateDefaults()

src/Forms/FormRequestHandler.php

@@ -137,7 +137,6 @@ class FormRequestHandler extends RequestHandler
         $this->form->loadDataFrom($vars, true, $allowedFields);
 
         // Protection against CSRF attacks
-        // @todo Move this to SecurityTokenField::validate()
         $token = $this->form->getSecurityToken();
         if (! $token->checkRequest($request)) {
             $securityID = $token->getName();

src/Forms/FormScaffolder.php

@@ -36,16 +36,12 @@ class FormScaffolder
     /**
      * @var array $restrictFields Numeric array of a field name whitelist.
      * If left blank, all fields from {@link DataObject->db()} will be included.
-     *
-     * @todo Implement restrictions for has_many and many_many relations.
      */
     public $restrictFields;
 
     /**
      * @var array $fieldClasses Optional mapping of fieldnames to subclasses of {@link FormField}.
      * By default the scaffolder will determine the field instance by {@link DBField::scaffoldFormField()}.
-     *
-     * @todo Implement fieldClasses for has_many and many_many relations
      */
     public $fieldClasses;
 
@@ -87,7 +83,6 @@ class FormScaffolder
                 continue;
             }
 
-            // @todo Pass localized title
             if ($this->fieldClasses && isset($this->fieldClasses[$fieldName])) {
                 $fieldClass = $this->fieldClasses[$fieldName];
                 $fieldObject = new $fieldClass($fieldName);

src/Forms/GridField/GridField.php

@@ -342,8 +342,6 @@ class GridField extends FormField
     /**
      * Cast an arbitrary value with the help of a $castingDefinition.
      *
-     * @todo refactor this into GridFieldComponent
-     *
      * @param mixed $value
      * @param string|array $castingDefinition
      *
@@ -555,8 +553,6 @@ class GridField extends FormField
         ];
         $fragmentDeferred = [];
 
-        // TODO: Break the below into separate reducer methods
-
         // Continue looping if any placeholders exist
         while (array_filter($content ?? [], function ($value) {
             return preg_match(self::FRAGMENT_REGEX ?? '', $value ?? '');
@@ -1182,8 +1178,6 @@ class GridField extends FormField
      * Custom request handler that will check component handlers before proceeding to the default
      * implementation.
      *
-     * @todo copy less code from RequestHandler.
-     *
      * @param HTTPRequest $request
      * @return array|RequestHandler|HTTPResponse|string
      * @throws HTTPResponse_Exception

src/Forms/GridField/GridFieldDetailForm_ItemRequest.php

@@ -169,7 +169,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler
         } else {
             // If not requested by ajax, we need to render it within the controller context+template
             return $controller->customise([
-                // TODO CMS coupling
                 'Content' => $return,
             ]);
         }
@@ -179,10 +178,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler
      * Builds an item edit form.  The arguments to getCMSFields() are the popupController and
      * popupFormName, however this is an experimental API and may change.
      *
-     * @todo In the future, we will probably need to come up with a tigher object representing a partially
-     * complete controller with gaps for extra functionality.  This, for example, would be a better way
-     * of letting Security/login put its log-in form inside a UI specified elsewhere.
-     *
      * @return Form|HTTPResponse
      */
     public function ItemEditForm()
@@ -266,12 +261,10 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler
             $form->loadDataFrom(['ManyMany' => $extraData]);
         }
 
-        // TODO Coupling with CMS
         $toplevelController = $this->getToplevelController();
         if ($toplevelController && $toplevelController instanceof LeftAndMain) {
             // Always show with base template (full width, no other panels),
             // regardless of overloaded CMS controller templates.
-            // TODO Allow customization, e.g. to display an edit form alongside a search form from the CMS controller
             $form->setTemplate([
                 'type' => 'Includes',
                 'SilverStripe\\Admin\\LeftAndMain_EditForm',
@@ -464,7 +457,6 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler
 
     protected function getBackLink()
     {
-        // TODO Coupling with CMS
         $backlink = '';
         $toplevelController = $this->getToplevelController();
         if ($toplevelController && $toplevelController instanceof LeftAndMain) {

src/Forms/GridField/GridFieldExportButton.php

@@ -209,7 +209,6 @@ class GridFieldExportButton extends AbstractGridFieldComponent implements GridFi
 
         $items = $gridField->getManipulatedList();
 
-        // @todo should GridFieldComponents change behaviour based on whether others are available in the config?
         foreach ($gridField->getConfig()->getComponents() as $component) {
             if ($component instanceof GridFieldFilterHeader || $component instanceof GridFieldSortableHeader) {
                 $items = $component->getManipulatedData($gridField, $items);

src/Forms/GroupedDropdownField.php

@@ -55,8 +55,6 @@ use SilverStripe\View\ArrayData;
 class GroupedDropdownField extends DropdownField
 {
 
-    // TODO remove this when GroupedDropdownField is implemented
-    // This should be one of SCHEMA_DATA_TYPE_* constants instead
     protected $schemaDataType = 'GroupedDropdownField';
 
     /**

src/Forms/MoneyField.php

@@ -17,7 +17,6 @@ use SilverStripe\ORM\DataObjectInterface;
 class MoneyField extends FormField
 {
 
-    // TODO replace with `FormField::SCHEMA_DATA_TYPE_TEXT` when MoneyField is implemented
     protected $schemaDataType = 'MoneyField';
 
     /**

src/Forms/SelectionGroup.php

@@ -90,7 +90,6 @@ class SelectionGroup extends CompositeField
             }
 
             $itemID = $this->ID() . '_' . (++$count);
-            // @todo Move into SelectionGroup_Item.ss template at some point.
             $extra = [
                 "RadioButton" => DBField::create_field('HTMLFragment', HTML::createTag(
                     'input',

src/Forms/TimeField.php

@@ -14,8 +14,6 @@ use SilverStripe\ORM\FieldType\DBTime;
  * # Localization
  *
  * See {@link DateField}
- *
- * @todo Timezone support
  */
 class TimeField extends TextField
 {

src/Logging/DebugViewFriendlyErrorFormatter.php

@@ -126,7 +126,6 @@ class DebugViewFriendlyErrorFormatter implements FormatterInterface
      */
     public function output($statusCode)
     {
-        // TODO: Refactor into a content-type option
         if (Director::is_ajax()) {
             return $this->getTitle();
         }

src/Logging/HTTPOutputHandler.php

@@ -165,8 +165,6 @@ class HTTPOutputHandler extends AbstractProcessingHandler
             }
         }
 
-        // TODO: This coupling isn't ideal
-        // See https://github.com/silverstripe/silverstripe-framework/issues/4484
         if (Controller::has_curr()) {
             $response = Controller::curr()->getResponse();
         } else {

src/ORM/Connect/DBConnector.php

@@ -37,7 +37,6 @@ abstract class DBConnector
      * it will be called on the object itself and as such can be overridden in a subclass.
      * Subclasses should run all errors through this function.
      *
-     * @todo hook this into a more well-structured error handling system.
      * @param string $msg The error message.
      * @param integer $errorLevel The level of the error to throw.
      * @param string $sql The SQL related to this query

src/ORM/Connect/DBSchemaManager.php

@@ -350,8 +350,6 @@ abstract class DBSchemaManager
      * Generate the following table in the database, modifying whatever already exists
      * as necessary.
      *
-     * @todo Change detection for CREATE TABLE $options other than "Engine"
-     *
      * @param string $table The name of the table
      * @param array $fieldSchema A list of the fields to create, in the same form as DataObject::$db
      * @param array $indexSchema A list of indexes to create. See {@link requireIndex()}
@@ -669,7 +667,6 @@ abstract class DBSchemaManager
      */
     public function requireField($table, $field, $spec)
     {
-        //TODO: this is starting to get extremely fragmented.
         //There are two different versions of $spec floating around, and their content changes depending
         //on how they are structured.  This needs to be tidied up.
         $fieldValue = null;
@@ -687,7 +684,6 @@ abstract class DBSchemaManager
 
         // Collations didn't come in until MySQL 4.1.  Anything earlier will throw a syntax error if you try and use
         // collations.
-        // TODO: move this to the MySQLDatabase file, or drop it altogether?
         if (!$this->database->supportsCollations()) {
             $spec = preg_replace('/ *character set [^ ]+( collate [^ ]+)?( |$)/', '\\2', $spec ?? '');
         }
@@ -974,7 +970,6 @@ abstract class DBSchemaManager
      * @param string $tableName The name of the table.
      * @param string $indexName The name of the index.
      * @param array $indexSpec The specification of the index, see Database::requireIndex() for more details.
-     * @todo Find out where this is called from - Is it even used? Aren't indexes always dropped and re-added?
      */
     abstract public function alterIndex($tableName, $indexName, $indexSpec);
 

src/ORM/Connect/Database.php

@@ -563,7 +563,6 @@ abstract class Database
      */
     public function concatOperator()
     {
-        // @todo Make ' + ' in mssql
         return ' || ';
     }
 
@@ -714,7 +713,6 @@ abstract class Database
      * will be an extension name, and the value the configuration for that extension. This
      * could be one of partitions, tablespaces, or clustering
      * @return boolean Flag indicating support for all of the above
-     * @todo Write test cases
      */
     public function supportsExtensions($extensions)
     {

src/ORM/DB.php

@@ -471,8 +471,6 @@ class DB
      * That's a limitation of the system that's due to it being written for {@link DataObject::write()},
      * which needs to do a single write on a number of different tables.
      *
-     * @todo Update this to support paramaterised queries
-     *
      * @param array $manipulation
      */
     public static function manipulate($manipulation)

src/ORM/DataList.php

@@ -472,8 +472,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * E.g. ->filter('Field:not', 'value) will generate '... OR "Field" IS NULL', and
      * ->filter('Field:not', null) will generate '"Field" IS NOT NULL'
      *
-     * @todo extract the sql from $customQuery into a SQLGenerator class
-     *
      * @param string|array Escaped SQL statement. If passed as array, all keys and values will be escaped internally
      * @return $this
      */
@@ -536,8 +534,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      *          $list = $list->filterAny(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43)));
      *          // SQL: WHERE (("Name" IN ('bob', 'phil')) OR ("Age" IN ('21', '43'))
      *
-     * @todo extract the sql from this method into a SQLGenerator class
-     *
      * @param string|array See {@link filter()}
      * @return static
      */
@@ -679,8 +675,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * @example $list = $list->exclude(array('Name'=>array('bob','phil'), 'Age'=>array(21, 43)));
      *          // bob age 21 or 43, phil age 21 or 43 would be excluded
      *
-     * @todo extract the sql from this method into a SQLGenerator class
-     *
      * @param string|array
      * @param string [optional]
      *
@@ -1817,7 +1811,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
     /**
      * Return a new item to add to this DataList.
      *
-     * @todo This doesn't factor in filters.
      * @param array $initialFields
      * @return DataObject
      */
@@ -1831,8 +1824,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * Remove this item by deleting it
      *
      * @param DataObject $item
-     * @todo Allow for amendment of this behaviour - for example, we can remove an item from
-     * an "ActiveItems" DataList by changing the status to inactive.
      */
     public function remove($item)
     {

src/ORM/DataObject.php

@@ -99,9 +99,6 @@ use stdClass;
  * the results are cached in memory through {@link cachedCall()}.
  *
  *
- * @todo Add instance specific removeExtension() which undos loadExtraStatics()
- *  and defineMethods()
- *
  * @property int $ID ID of the DataObject, 0 if the DataObject doesn't exist in database.
  * @property int $OldID ID of object, if deleted
  * @property string $Title
@@ -128,8 +125,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
     private static $plural_name = null;
 
     /**
-     * Allow API access to this object?
-     * @todo Define the options that can be set here
      * @config
      */
     private static $api_access = false;
@@ -682,8 +677,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
         $clonedItem = $item->duplicate(false);
         $destinationObject->setComponent($relation, $clonedItem);
         // After $clonedItem is assigned the appropriate FieldID / FieldClass, force write
-        // @todo Write this component in onAfterWrite instead, assigning the FieldID then
-        // https://github.com/silverstripe/silverstripe-framework/issues/7818
         $clonedItem->write();
     }
 
@@ -1151,7 +1144,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
                 continue;
             }
 
-            // TODO remove redundant merge of has_one fields
             $leftObj->{$key} = $rightObj->{$key};
         }
 
@@ -1752,10 +1744,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
             throw new LogicException("DataObject::delete() called on a DataObject without an ID");
         }
 
-        // TODO: This is quite ugly.  To improve:
-        //  - move the details of the delete code in the DataQuery system
-        //  - update the code to just delete the base table, and rely on cascading deletes in the DB to do the rest
-        //    obviously, that means getting requireTable() to configure cascading deletes ;-)
         $srcQuery = DataList::create(static::class)
             ->filter('ID', $this->ID)
             ->dataQuery()
@@ -1908,8 +1896,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
         $schema = static::getSchema();
         if ($class = $schema->hasOneComponent(static::class, $componentName)) {
             // Force item to be written if not by this point
-            // @todo This could be lazy-written in a beforeWrite hook, but force write here for simplicity
-            // https://github.com/silverstripe/silverstripe-framework/issues/7818
             if ($item && !$item->isInDB()) {
                 $item->write();
             }
@@ -2594,8 +2580,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      * by default. To customize, either overload this method in your
      * subclass, or extend it by {@link DataExtension->updateFrontEndFields()}.
      *
-     * @todo Decide on naming for "website|frontend|site|page" and stick with it in the API
-     *
      * @param array $params See {@link scaffoldFormFields()}
      * @return FieldList Always returns a simple field collection without TabSet.
      */
@@ -2692,7 +2676,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
         $columns = [];
 
         // Add SQL for fields, both simple & multi-value
-        // TODO: This is copy & pasted from buildSQL(), it could be moved into a method
         $databaseFields = $schema->databaseFields($class, false);
         foreach ($databaseFields as $k => $v) {
             if (!isset($this->record[$k]) || $this->record[$k] === null) {
@@ -2771,7 +2754,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
                 array_keys($this->record ?? []),
                 array_fill(0, count($this->record ?? []), self::CHANGE_STRICT)
             );
-            // @todo Find better way to allow versioned to write a new version after forceChange
             unset($changed['Version']);
         } else {
             $changed = $this->changed;
@@ -2905,8 +2887,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 
         // if a field is not existing or has strictly changed
         if (!array_key_exists($fieldName, $this->original ?? []) || $this->original[$fieldName] !== $val) {
-            // TODO Add check for php-level defaults which are not set in the db
-            // TODO Add check for hidden input-fields (readonly) which are not set in the db
             // At the very least, the type has changed
             $this->changed[$fieldName] = self::CHANGE_STRICT;
 
@@ -3314,8 +3294,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      * @param string|array $limit A limit expression to be inserted into the LIMIT clause.
      * @param string $containerClass The container class to return the results in.
      *
-     * @todo $containerClass is Ignored, why?
-     *
      * @return DataList The objects matching the filter, in the class specified by $containerClass
      */
     public static function get(
@@ -3480,7 +3458,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      */
     public static function reset()
     {
-        // @todo Decouple these
         DBEnum::flushCache();
         ClassInfo::reset_db_cache();
         static::getSchema()->reset();
@@ -3941,8 +3918,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
     /**
      * Get the default summary fields for this object.
      *
-     * @todo use the translation apparatus to return a default field selection for the language
-     *
      * @return array
      */
     public function summaryFields()
@@ -4000,8 +3975,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
      * it is constructed here. Otherwise, the default filter specified in
      * {@link DBField} is used.
      *
-     * @todo error handling/type checking for valid FormField and SearchFilter subclasses?
-     *
      * @return array
      */
     public function defaultSearchFilters()

src/ORM/DataObjectSchema.php

@@ -1153,7 +1153,7 @@ class DataObjectSchema
             if ($key === 'from') {
                 return $relationClass;
             }
-            // @todo support polymorphic 'to'
+
             throw new InvalidArgumentException(
                 "many_many through relation {$parentClass}.{$component} {$key} references a polymorphic field "
                 . "{$joinClass}::{$relation} which is not supported"

src/ORM/DataQuery.php

@@ -63,9 +63,8 @@ class DataQuery
      */
     private $queryFinalised = false;
 
-    // TODO: replace subclass_access with this
     protected $querySubclasses = true;
-    // TODO: replace restrictclasses with this
+
     protected $filterByClassName = true;
 
     /**
@@ -335,9 +334,6 @@ class DataQuery
             "RecordClassName"
         );
 
-        // TODO: Versioned, SiteTreeSubsites, etc, could probably be better implemented as subclasses
-        // of DataQuery
-
         $obj = Injector::inst()->get($this->dataClass);
         $obj->extend('augmentSQL', $query, $this);
 

src/ORM/DatabaseAdmin.php

@@ -519,8 +519,6 @@ class DatabaseAdmin extends Controller
 
     /**
      * Migrate all class names
-     *
-     * @todo Migrate to separate build task
      */
     protected function migrateClassNames()
     {

src/ORM/FieldType/DBComposite.php

@@ -12,8 +12,6 @@ use SilverStripe\ORM\Queries\SQLSelect;
  * Extend this class when designing a {@link DBField} that doesn't have a 1-1 mapping with a database field.
  * This includes multi-value fields and transformed fields
  *
- * @todo Unittests for loading and saving composite values (see GIS module for existing similar unittests)
- *
  * Example with a combined street name and number:
  * <code>
 * class Street extends DBComposite {

src/ORM/FieldType/DBDatetime.php

@@ -30,7 +30,6 @@ use SilverStripe\View\TemplateGlobalProvider;
  * );
  * </code>
  *
- * @todo Add localization support, see http://open.silverstripe.com/ticket/2931
  */
 class DBDatetime extends DBDate implements TemplateGlobalProvider
 {

src/ORM/FieldType/DBDouble.php

@@ -14,7 +14,6 @@ class DBDouble extends DBFloat
     public function requireField()
     {
         // HACK: MSSQL does not support double so we're using float instead
-        // @todo This should go into MSSQLDatabase ideally somehow
         if (DB::get_conn() instanceof MySQLDatabase) {
             DB::require_field($this->tableName, $this->name, "double");
         } else {

src/ORM/FieldType/DBField.php

@@ -41,8 +41,6 @@ use SilverStripe\View\ViewableData;
  *  }
  * }
  * </code>
- *
- * @todo remove MySQL specific code from subclasses
  */
 abstract class DBField extends ViewableData implements DBIndexable
 {
@@ -579,12 +577,6 @@ abstract class DBField extends ViewableData implements DBIndexable
     }
 
     /**
-     * @todo documentation
-     *
-     * @todo figure out how we pass configuration parameters to
-     *       search filters (note: parameter hack now in place to pass in the required full path - using $this->name
-     *       won't work)
-     *
      * @param string $name Override name of this field
      * @return SearchFilter
      */

src/ORM/FieldType/DBLocale.php

@@ -6,8 +6,6 @@ use SilverStripe\i18n\i18n;
 
 /**
  * Locale database field
- *
- * @todo Allowing showing locale values in different languages through Nice()
  */
 class DBLocale extends DBVarchar
 {

src/ORM/FieldType/DBMultiEnum.php

@@ -35,7 +35,6 @@ class DBMultiEnum extends DBEnum
 
     public function requireField()
     {
-        // @todo: Remove mysql-centric logic from this
         $charset = Config::inst()->get(MySQLDatabase::class, 'charset');
         $collation = Config::inst()->get(MySQLDatabase::class, 'collation');
         $values = [

src/ORM/FieldType/DBPolymorphicForeignKey.php

@@ -18,9 +18,8 @@ class DBPolymorphicForeignKey extends DBComposite
 
     public function scaffoldFormField($title = null, $params = null)
     {
-        // Opt-out of form field generation - Scaffolding should be performed on
+        // Don't provide scaffolded form field generation - Scaffolding should be performed on
         // the has_many end, or set programmatically.
-        // @todo - Investigate suitable FormField
         return null;
     }
 

src/ORM/FieldType/DBPrimaryKey.php

@@ -7,8 +7,6 @@ use SilverStripe\ORM\DB;
 
 /**
  * A special type Int field used for primary keys.
- *
- * @todo Allow for custom limiting/filtering of scaffoldFormField dropdown
  */
 class DBPrimaryKey extends DBInt
 {

src/ORM/Filters/ExactMatchFilter.php

@@ -13,7 +13,6 @@ use SilverStripe\ORM\DataList;
 /**
  * Selects textual content with an exact match between columnname and keyword.
  *
- * @todo documentation
  */
 class ExactMatchFilter extends SearchFilter
 {

src/ORM/Filters/FulltextFilter.php

@@ -25,7 +25,6 @@ use Exception;
  *   ];
  * </code>
  *
- * @todo Add support for databases besides MySQL
  */
 class FulltextFilter extends SearchFilter
 {

src/ORM/Filters/SearchFilter.php

@@ -332,7 +332,6 @@ abstract class SearchFilter
     public function getDbFormattedValue()
     {
         // SRM: This code finds the table where the field named $this->name lives
-        // Todo: move to somewhere more appropriate, such as DataMapper, the magical class-to-be?
 
         if ($this->aggregate) {
             return intval($this->value);

src/ORM/Filters/WithinRangeFilter.php

@@ -4,11 +4,6 @@ namespace SilverStripe\ORM\Filters;
 
 use SilverStripe\ORM\DataQuery;
 
-/**
- * Incomplete.
- *
- * @todo add to tests
- */
 class WithinRangeFilter extends SearchFilter
 {
 

src/ORM/HasManyList.php

@@ -119,7 +119,6 @@ class HasManyList extends RelationList
      * Doesn't actually remove the item, it just clears the foreign key value.
      *
      * @param DataObject $item The DataObject to be removed
-     * @todo Maybe we should delete the object instead?
      */
     public function remove($item)
     {

src/ORM/PolymorphicHasManyList.php

@@ -111,7 +111,6 @@ class PolymorphicHasManyList extends HasManyList
      * Doesn't actually remove the item, it just clears the foreign key value.
      *
      * @param DataObject $item The DataObject to be removed
-     * @todo Maybe we should delete the object instead?
      */
     public function remove($item)
     {

src/ORM/Queries/SQLAssignmentRow.php

@@ -68,8 +68,6 @@ class SQLAssignmentRow
                     $parameters = [$parameters];
                 }
 
-                // @todo Some input sanitisation checking the key contains the
-                // correct number of ? placeholders as the number of parameters
                 return [$sql => $parameters];
             }
         }

src/ORM/Queries/SQLConditionalExpression.php

@@ -256,8 +256,6 @@ abstract class SQLConditionalExpression extends SQLExpression
     /**
      * Retrieves the finalized list of joins
      *
-     * @todo This part of the code could be simplified
-     *
      * @param array $parameters Out variable for parameters required for this query
      * @return array List of joins as a mapping from array('Alias' => 'Join Expression')
      */
@@ -693,8 +691,6 @@ abstract class SQLConditionalExpression extends SQLExpression
     /**
      * Checks whether this query is for a specific ID in a table
      *
-     * @todo Doesn't work with combined statements (e.g. "Foo='bar' AND ID=5")
-     *
      * @return boolean
      */
     public function filtersOnID()
@@ -713,15 +709,12 @@ abstract class SQLConditionalExpression extends SQLExpression
     /**
      * Checks whether this query is filtering on a foreign key, ie finding a has_many relationship
      *
-     * @todo Doesn't work with combined statements (e.g. "Foo='bar' AND ParentID=5")
-     *
      * @return boolean
      */
     public function filtersOnFK()
     {
         $regexp = '/^(.*\.)?("|`)?[a-zA-Z]+ID("|`)?\s?(=|IN)/';
 
-        // @todo - Test this works with parameterized queries
         foreach ($this->getWhereParameterised($parameters) as $predicate) {
             if (preg_match($regexp ?? '', $predicate ?? '')) {
                 return true;

src/ORM/Queries/SQLExpression.php

@@ -45,8 +45,6 @@ abstract class SQLExpression
     /**
      * Return the generated SQL string for this query
      *
-     * @todo Is it ok for this to consider parameters? Test cases here!
-     *
      * @return string
      */
     public function __toString()

src/ORM/Queries/SQLSelect.php

@@ -575,7 +575,6 @@ class SQLSelect extends SQLConditionalExpression
         // Choose a default column
         if ($column == null) {
             if ($this->groupby) {
-                // @todo Test case required here
                 $countQuery = new SQLSelect();
                 $countQuery->setSelect("count(*)");
                 $countQuery->setFrom(['(' . $clone->sql($innerParameters) . ') all_distinct']);

src/ORM/Search/SearchContext.php

@@ -107,10 +107,6 @@ class SearchContext
         return ($this->fields) ? $this->fields : singleton($this->modelClass)->scaffoldSearchFields();
     }
 
-    /**
-     * @todo move to SQLSelect
-     * @todo fix hack
-     */
     protected function applyBaseTableFields()
     {
         $classes = ClassInfo::dataClassesFor($this->modelClass);
@@ -327,8 +323,6 @@ class SearchContext
     /**
      * Returns a result set from the given search parameters.
      *
-     * @todo rearrange start and limit params to reflect DataObject
-     *
      * @param array $searchParams
      * @param array|bool|string $sort
      * @param array|null|string $limit

src/Security/Group.php

@@ -183,8 +183,6 @@ class Group extends DataObject
                         if ($record && !$record->ID) {
                             $groupsField->setValue($group->ID);
                         } elseif ($record && $record->ID) {
-                            // TODO Mark disabled once chosen.js supports it
-                            // $groupsField->setDisabledItems(array($group->ID));
                             $form->Fields()->replaceField(
                                 'DirectGroups',
                                 $groupsField->performReadonlyTransformation()
@@ -194,8 +192,6 @@ class Group extends DataObject
                 });
             $memberList = GridField::create('Members', false, $this->DirectMembers(), $config)
                 ->addExtraClass('members_grid');
-            // @todo Implement permission checking on GridField
-            //$memberList->setPermissions(array('edit', 'delete', 'export', 'add', 'inlineadd'));
             $fields->addFieldToTab('Root.Members', $memberList);
         }
 
@@ -238,8 +234,6 @@ class Group extends DataObject
                     sprintf(
                         '<a href="%s" class="add-role">%s</a>',
                         SecurityAdmin::singleton()->Link('show/root#Root_Roles'),
-                        // TODO This should include #Root_Roles to switch directly to the tab,
-                        // but tabstrip.js doesn't display tabs when directly addressed through a URL pragma
                         _t('SilverStripe\\Security\\Group.RolesAddEditLink', 'Manage roles')
                     ) .
                     "</p>"

src/Security/GroupCsvBulkLoader.php

@@ -5,9 +5,6 @@ namespace SilverStripe\Security;
 use SilverStripe\Dev\CsvBulkLoader;
 use SilverStripe\ORM\DataObject;
 
-/**
- * @todo Migrate Permission->Arg and Permission->Type values
- */
 class GroupCsvBulkLoader extends CsvBulkLoader
 {
 

src/Security/Member.php

@@ -114,9 +114,6 @@ class Member extends DataObject
      * statement.
      *
      * @var array
-     * @todo Generic implementation of $searchable_fields on DataObject,
-     * with definition for different searching algorithms
-     * (LIKE, FULLTEXT) and default FormFields to construct a searchform.
      */
     private static $searchable_fields = [
         'FirstName',
@@ -446,7 +443,6 @@ class Member extends DataObject
      */
     public function beforeMemberLoggedIn()
     {
-        // @todo Move to middleware on the AuthenticationMiddleware IdentityStore
         $this->extend('beforeMemberLoggedIn');
     }
 
@@ -619,7 +615,6 @@ class Member extends DataObject
      * Returns the fields for the member form - used in the registration/profile module.
      * It should return fields that are editable by the admin and the logged-in user.
      *
-     * @todo possibly move this to an extension
      *
      * @return FieldList Returns a {@link FieldList} containing the fields for
      *                   the member form.
@@ -770,7 +765,6 @@ class Member extends DataObject
 
         // We don't send emails out on dev/tests sites to prevent accidentally spamming users.
         // However, if TestMailer is in use this isn't a risk.
-        // @todo some developers use external tools, so emailing might be a good idea anyway
         if ((Director::isLive() || Injector::inst()->get(MailerInterface::class) instanceof TestMailer)
             && $this->isChanged('Password')
             && $this->record['Password']
@@ -1151,7 +1145,6 @@ class Member extends DataObject
      * including any parent groups where membership is implied.
      * Use {@link DirectGroups()} to only retrieve the group relations without inheritance.
      *
-     * @todo Push all this logic into Member_GroupSet's getIterator()?
      * @return Member_Groupset
      */
     public function Groups()

src/Security/MemberAuthenticator/ChangePasswordHandler.php

@@ -58,7 +58,6 @@ class ChangePasswordHandler extends RequestHandler
 
     /**
      * Handle the change password request
-     * @todo this could use some spring cleaning
      *
      * @return array|HTTPResponse
      */

src/Security/MemberAuthenticator/CookieAuthenticationHandler.php

@@ -173,13 +173,9 @@ class CookieAuthenticationHandler implements AuthenticationHandler
         }
 
         if ($this->cascadeInTo) {
-            // @todo look at how to block "regular login" triggers from happening here
-            // @todo deal with the fact that the Session::current_session() isn't correct here :-/
             $this->cascadeInTo->logIn($member, false, $request);
         }
 
-        // @todo Consider whether response should be part of logIn() as well
-
         // Renew the token
         $rememberLoginHash->renew();
         $tokenExpiryDays = RememberLoginHash::config()->uninherited('token_expiry_days');

src/Security/MemberAuthenticator/MemberAuthenticator.php

@@ -168,7 +168,6 @@ class MemberAuthenticator implements Authenticator
 
     /**
      * Log login attempt
-     * TODO We could handle this with an extension
      *
      * @param array $data
      * @param HTTPRequest $request

src/Security/MemberAuthenticator/MemberLoginForm.php

@@ -84,7 +84,6 @@ class MemberLoginForm extends BaseLoginForm
         }
 
         if ($checkCurrentUser && Security::getCurrentUser()) {
-            // @todo find a more elegant way to handle this
             $logoutAction = Security::logout_url();
             $fields = FieldList::create(
                 HiddenField::create('AuthenticationMethod', null, $this->getAuthenticatorClass(), $this)
@@ -134,7 +133,6 @@ class MemberLoginForm extends BaseLoginForm
             HiddenField::create("AuthenticationMethod", null, $this->getAuthenticatorClass(), $this),
             // Regardless of what the unique identifier field is (usually 'Email'), it will be held in the
             // 'Email' value, below:
-            // @todo Rename the field to a more generic covering name
             $emailField = TextField::create("Email", $label, null, null, $this),
             PasswordField::create("Password", _t('SilverStripe\\Security\\Member.PASSWORD', 'Password'))
         );

src/Security/MemberAuthenticator/SessionAuthenticationHandler.php

@@ -80,7 +80,6 @@ class SessionAuthenticationHandler implements AuthenticationHandler
         $session->set($this->getSessionVariable(), $member->ID);
 
         // This lets apache rules detect whether the user has logged in
-        // @todo make this a setting on the authentication handler
         if (Member::config()->get('login_marker_cookie')) {
             Cookie::set(Member::config()->get('login_marker_cookie'), 1, 0);
         }
@@ -109,7 +108,6 @@ class SessionAuthenticationHandler implements AuthenticationHandler
         $file = '';
         $line = '';
 
-        // TODO: deprecate and use Session::regenerateSessionId
         // @ is to suppress win32 warnings/notices when session wasn't cleaned up properly
         // There's nothing we can do about this, because it's an operating system function!
         if (!headers_sent($file, $line)) {

src/Security/MemberCsvBulkLoader.php

@@ -45,7 +45,6 @@ class MemberCsvBulkLoader extends CsvBulkLoader
         /** @var Member $member */
         $member = DataObject::get_by_id($this->objectClass, $objID);
         foreach ($this->groups as $group) {
-            // TODO This isnt the most memory effective way to add members to a group
             $member->Groups()->add($group);
         }
 

src/Security/Permission.php

@@ -133,7 +133,6 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
 
     /**
      * Flush the permission cache, for example if you have edited group membership or a permission record.
-     * @todo Call this whenever Group_Members is added to or removed from
      */
     public static function reset()
     {

src/Security/Security.php

@@ -416,7 +416,6 @@ class Security extends Controller implements TemplateGlobalProvider
             $request->getSession()->set("BackURL", $_SERVER['REQUEST_URI']);
         }
 
-        // TODO AccessLogEntry needs an extension to handle permission denied errors
         // Audit logging hook
         $controller->extend('permissionDenied', $member);
 

src/Security/SecurityToken.php

@@ -35,7 +35,6 @@ use SilverStripe\View\TemplateGlobalProvider;
  * }
  * </code>
  *
- * @todo Make token name form specific for additional forgery protection.
  */
 class SecurityToken implements TemplateGlobalProvider
 {

src/View/SSTemplateParser.php

@@ -1886,8 +1886,6 @@ class SSTemplateParser extends Parser implements TemplateParser
             $res['php'] .= '((bool)'.$sub['php'].')';
         } else {
             $php = ($sub['ArgumentMode'] == 'default' ? $sub['lookup_php'] : $sub['php']);
-            // TODO: kinda hacky - maybe we need a way to pass state down the parse chain so
-            // Lookup_LastLookupStep and Argument_BareWord can produce hasValue instead of XML_val
             $res['php'] .= str_replace('$$FINAL', 'hasValue', $php ?? '');
         }
     }
@@ -5292,8 +5290,6 @@ class SSTemplateParser extends Parser implements TemplateParser
         $text = stripslashes($text ?? '');
         $text = addcslashes($text ?? '', '\'\\');
 
-        // TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this
-        // non-dynamically calculated
         $code = <<<'EOC'
 (\SilverStripe\View\SSViewer::getRewriteHashLinksDefault()
     ? \SilverStripe\Core\Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) )
@@ -5332,8 +5328,7 @@ EOC;
 
             $this->includeDebuggingComments = $includeDebuggingComments;
 
-            // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF
+            // Ignore UTF8 BOM at beginning of string.
-            // (and other encodings) properly
             if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) {
                 $this->pos = 3;
             }

src/View/ViewableData.php

@@ -438,8 +438,6 @@ class ViewableData implements IteratorAggregate
     {
         $class = $this->castingClass($field) ?: $this->config()->get('default_cast');
 
-        // TODO: It would be quicker not to instantiate the object, but to merely
-        // get its class from the Injector
         /** @var DBField $type */
         $type = Injector::inst()->get($class, true);
         return $type->config()->get('escape_type');

src/i18n/TextCollection/i18nTextCollector.php

@@ -71,8 +71,6 @@ class i18nTextCollector
      * The directory base on which the collector should act.
      * Usually the webroot set through {@link Director::baseFolder()}.
      *
-     * @todo Fully support changing of basePath through {@link SSViewer} and {@link ManifestBuilder}
-     *
      * @var string
      */
     public $basePath;
@@ -314,9 +312,6 @@ class i18nTextCollector
             return $owner;
         }
 
-        // @todo - How to determine ownership of templates? Templates can
-        // exist in multiple locations with the same name.
-
         // Display notice if not found
         Debug::message(
             "Duplicate key {$key} detected in no / multiple modules with no obvious owner",

tests/behat/src/CmsFormsContext.php

@@ -137,7 +137,6 @@ class CmsFormsContext implements Context
 	 * Example: Given "my text" in the "Content" HTML field should be right aligned
 	 * Example: Given "my text" in the "Content" HTML field should not be bold
 	 *
-	 * @todo Use an actual DOM parser for more accurate assertions
 	 *
 	 * @Given /^"(?P<text>([^"]*))" in the "(?P<field>(?:[^"]|\\")*)" HTML field should(?P<negate>(?: not)?) be (?P<formatting>(.*))$/
 	 */
@@ -194,8 +193,7 @@ class CmsFormsContext implements Context
         $text = addcslashes($text ?? '', "'");
 
         $js = <<<JS
-// TODO <IE9 support
+
-// TODO Allow text matches across nodes
 var editor = jQuery('#$inputFieldId').entwine('ss').getEditor(),
 	doc = editor.getInstance().getDoc(),
 	sel = editor.getInstance().selection,

tests/behat/src/CmsUiContext.php

@@ -594,8 +594,6 @@ SCRIPT
      */
     public function clickLinkInPreview($link)
     {
-        // TODO Remove once we have native support in Mink and php-webdriver,
-        // see https://groups.google.com/forum/#!topic/behat/QNhOuGHKEWI
         $this->getSession()->switchToIFrame('cms-preview-iframe');
         $link = $this->fixStepArgument($link);
         $this->getSession()->getPage()->clickLink($link);

tests/php/Control/DirectorTest.php

@@ -18,9 +18,6 @@ use SilverStripe\Core\Environment;
 use SilverStripe\Core\Kernel;
 use SilverStripe\Dev\SapphireTest;
 
-/**
- * @todo test Director::alternateBaseFolder()
- */
 class DirectorTest extends SapphireTest
 {
     protected static $extra_controllers = [

tests/php/Control/SessionTest.php

@@ -133,9 +133,6 @@ class SessionTest extends SapphireTest
     public function testStartRetainsInMemoryData()
     {
         $this->markTestIncomplete('Test');
-        // TODO Figure out how to simulate session vars without a session_start() resetting them
-        // $_SESSION['existing'] = true;
-        // $_SESSION['merge'] = 1;
         $req = new HTTPRequest('GET', '/');
         $session = new Session(null); // unstarted session
         $session->set('new', true);

tests/php/Core/ConvertTest.php

@@ -214,8 +214,6 @@ PHP
 
     /**
      * Tests {@link Convert::testRaw2URL()}
-     *
-     * @todo test toASCII()
      */
     public function testRaw2URL()
     {

tests/php/Core/Manifest/ThemeResourceLoaderTest.php

@@ -213,7 +213,6 @@ class ThemeResourceLoaderTest extends SapphireTest
      */
     public function testFindTemplatesInApplication()
     {
-        // TODO: replace with one that doesn't create temporary files (so bad)
         $templates = [
             $this->base . '/myproject/templates/Page.ss',
             $this->base . '/myproject/templates/Layout/Page.ss'

tests/php/Core/ObjectTest.php

@@ -22,11 +22,6 @@ use SilverStripe\Core\Tests\ObjectTest\TestExtension;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Versioned\Versioned;
 
-/**
- * @todo tests for addStaticVars()
- * @todo tests for setting statics which are not defined on the object as built-in PHP statics
- * @todo tests for setting statics through extensions (#2387)
- */
 class ObjectTest extends SapphireTest
 {
 
@@ -253,8 +248,6 @@ class ObjectTest extends SapphireTest
             "Injected sub-extensions are detected with static has_extension() when added through add_extension()"
         );
 
-        // @todo At the moment, this does NOT remove the extension due to parameterized naming,
-        //  meaning the extension will remain added in further test cases
         ExtensionTest::remove_extension(ExtendTest3::class);
     }
 

tests/php/Core/PhpSyntaxTest.php

@@ -18,7 +18,6 @@ class PhpSyntaxTest extends SapphireTest
     public function testShortTagsOffWillWork()
     {
         // Ignore this test completely if running the test suite on windows
-        // TODO: Make it work on all platforms, by building an alternative to find | grep.
         $returnCode = 0;
         $output = [];
         exec("which find && which grep && which php", $output, $returnCode);
@@ -51,7 +50,6 @@ class PhpSyntaxTest extends SapphireTest
 
     public function getAllFiles($ext = 'php')
     {
-        // TODO: Unix only
         $cmd = sprintf(
             'find %s | grep %s',
             BASE_PATH,

tests/php/Dev/CsvBulkLoaderTest.php

@@ -227,8 +227,6 @@ class CsvBulkLoaderTest extends SapphireTest
 
     /**
      * Test import with custom identifiers by importing the data.
-     *
-     * @todo Test duplicateCheck callbacks
      */
     public function testLoadWithIdentifiers()
     {

tests/php/Forms/CurrencyFieldDisabledTest.php

@@ -18,9 +18,6 @@ class CurrencyFieldDisabledTest extends SapphireTest
         $this->assertStringContainsString('$5.00', $result, 'The value should be rendered');
     }
 
-    /**
-     * @todo: Update the expectation when intl for currencies is implemented
-     */
     public function testFieldWithCustomisedCurrencySymbol()
     {
         DBCurrency::config()->set('currency_symbol', '€');

tests/php/Forms/CurrencyFieldReadonlyTest.php

@@ -37,9 +37,6 @@ class CurrencyFieldReadonlyTest extends SapphireTest
         $this->assertStringContainsString('AUD0.00', $result, 'The value should be rendered');
     }
 
-    /**
-     * @todo: Update the expectation when intl for currencies is implemented
-     */
     public function testFieldWithCustomisedCurrencySymbol()
     {
         DBCurrency::config()->set('currency_symbol', '€');

tests/php/Forms/EmailFieldTest.php

@@ -18,8 +18,6 @@ class EmailFieldTest extends FunctionalTest
     /**
      * Check the php validator for email addresses. We should be checking against RFC 5322 which defines email address
      * syntax.
-     *
-     * @TODO
      *   - double quotes around the local part (before @) is not supported
      *   - special chars ! # $ % & ' * + - / = ? ^ _ ` { | } ~ are all valid in local part
      *   - special chars ()[]\;:,<> are valid in the local part if the local part is in double quotes

tests/php/Forms/FieldListTest.php

@@ -20,17 +20,6 @@ use SilverStripe\Forms\HiddenField;
 
 /**
  * Tests for FieldList
- *
- * @todo        test for {@link FieldList->setValues()}. Need to check
- *  that the values that were set are the correct ones given back.
- * @todo        test for {@link FieldList->transform()} and {@link FieldList->makeReadonly()}.
- *  Need to ensure that it correctly transforms the FieldList object.
- * @todo        test for {@link FieldList->HiddenFields()}. Need to check
- *  the fields returned are the correct HiddenField objects for a
- *  given FieldList instance.
- * @todo        test for {@link FieldList->dataFields()}.
- * @todo        test for {@link FieldList->findOrMakeTab()}.
- * @todo        the same as above with insertBefore() and insertAfter()
  */
 class FieldListTest extends SapphireTest
 {
@@ -856,9 +845,6 @@ class FieldListTest extends SapphireTest
         );
     }
 
-    /**
-     * @todo check actual placement of fields
-     */
     public function testInsertBeforeWithNestedTabsets()
     {
         $FieldListA = new FieldList(
@@ -973,9 +959,6 @@ class FieldListTest extends SapphireTest
         );
     }
 
-    /**
-     * @todo check actual placement of fields
-     */
     public function testInsertAfterWithNestedTabsets()
     {
         $FieldListA = new FieldList(

tests/php/Forms/GridField/GridFieldDetailFormTest/Category.php

@@ -24,7 +24,6 @@ class Category extends DataObject implements TestOnly
     public function getCMSFields()
     {
         $fields = parent::getCMSFields();
-        // TODO No longer necessary once FormScaffolder uses GridField
         $fields->replaceField(
             'People',
             GridField::create(

tests/php/Forms/GridField/GridFieldDetailFormTest/PeopleGroup.php

@@ -29,7 +29,6 @@ class PeopleGroup extends DataObject implements TestOnly
     public function getCMSFields()
     {
         $fields = parent::getCMSFields();
-        // TODO No longer necessary once FormScaffolder uses GridField
         $fields->replaceField(
             'People',
             GridField::create(

tests/php/Forms/GridField/GridFieldDetailFormTest/Person.php

@@ -39,7 +39,6 @@ class Person extends DataObject implements TestOnly
     public function getCMSFields()
     {
         $fields = parent::getCMSFields();
-        // TODO No longer necessary once FormScaffolder uses GridField
         $fields->replaceField(
             'Categories',
             GridField::create(

tests/php/Forms/GridField/GridFieldTest.php

@@ -185,10 +185,6 @@ class GridFieldTest extends SapphireTest
     {
         $obj = new GridField('testfield', 'testfield');
 
-        // @todo - PHP 7.0.6 change requires __isset() to return true
-        // for each reference from left to right along an isset() invocation.
-        // See https://bugs.php.net/bug.php?id=62059
-
         // Check value persistence
         $this->assertEquals(15, $obj->State->NoValue(15));
         $this->assertEquals(15, $obj->State->NoValue(-1));

tests/php/Forms/ListboxFieldTest.php

@@ -222,15 +222,6 @@ class ListboxFieldTest extends SapphireTest
             'Field validates values in source map'
         );
 
-        /**
-         * @todo re-enable these tests when field validation is removed from {@link ListboxField::setValue()} and moved
-         * to the {@link ListboxField::validate()} function
-         */
-        //      $field->setValue(4);
-        //      $this->assertFalse(
-        //          $field->validate($validator),
-        //          'Field does not validate values outside of source map'
-        //      );
         $field->setValue(
             false,
             new ArrayData(

tests/php/Forms/NumericFieldTest.php

@@ -128,9 +128,6 @@ class NumericFieldTest extends SapphireTest
         $field = new NumericField('Number');
 
         $html = $field->Field();
-
-        // @todo - Revert to number one day when html5 number supports proper localisation
-        // See https://github.com/silverstripe/silverstripe-framework/pull/4565
         $this->assertStringContainsString('type="text"', $html, 'number type not set');
     }
 

tests/php/Forms/RequiredFieldsTest.php

@@ -5,9 +5,6 @@ namespace SilverStripe\Forms\Tests;
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\Forms\RequiredFields;
 
-/**
- * @todo Test the validation method php()
- */
 class RequiredFieldsTest extends SapphireTest
 {
 

tests/php/Forms/TimeFieldTest.php

@@ -59,7 +59,6 @@ class TimeFieldTest extends SapphireTest
         $f = new TimeField('Time', 'Time');
         $f->setHTML5(false);
         $f->setLocale('fr_FR');
-        // TODO Find an hour format that's actually different
         $f->setValue('23:59');
         $this->assertEquals($f->dataValue(), '23:59:00');
     }

tests/php/ORM/DBFieldTest.php

@@ -117,8 +117,6 @@ class DBFieldTest extends SapphireTest
         $this->assertEquals(true, $boolean->prepValueForDB(1));
         $this->assertEquals(true, $boolean->prepValueForDB('1'));
 
-        // @todo - Revisit Varchar to evaluate correct behaviour of nullifyEmpty
-
         /* Varchar behaviour: nullifyifEmpty defaults to true */
         $varchar = DBVarchar::create();
         $this->assertEquals(0, $varchar->prepValueForDB(0));

tests/php/ORM/DataObjectLazyLoadingTest.php

@@ -95,8 +95,6 @@ class DataObjectLazyLoadingTest extends SapphireTest
         $teams = DataObject::get(Team::class); // query parent class
         $subteam1Lazy = $teams->find('ID', $subteam1->ID);
 
-        // TODO Fix hasField() to exclude *_Lazy
-        // $this->assertFalse($subteam1Lazy->hasField('SubclassDatabaseField_Lazy'));
         $this->assertTrue($subteam1Lazy->hasField('SubclassDatabaseField'));
     }
 

tests/php/ORM/DataObjectTest.php

@@ -830,9 +830,6 @@ class DataObjectTest extends SapphireTest
         $this->assertEquals($captain2->ID, $team1->Captain->ID);
     }
 
-    /**
-     * @todo Extend type change tests (e.g. '0'==NULL)
-     */
     public function testChangedFields()
     {
         $obj = $this->objFromFixture(DataObjectTest\Player::class, 'captain1');
@@ -1140,9 +1137,6 @@ class DataObjectTest extends SapphireTest
         );
     }
 
-    /**
-     * @todo Re-enable all test cases for field existence after behaviour has been fixed
-     */
     public function testFieldExistence()
     {
         $teamInstance = $this->objFromFixture(DataObjectTest\Team::class, 'team1');
@@ -1290,9 +1284,6 @@ class DataObjectTest extends SapphireTest
         );
     }
 
-    /**
-     * @todo Re-enable all test cases for field inheritance aggregation after behaviour has been fixed
-     */
     public function testFieldInheritance()
     {
         $schema = DataObject::getSchema();
@@ -1633,12 +1624,6 @@ class DataObjectTest extends SapphireTest
             null,
             'merge() with $overwriteWithEmpty overwrites empty fields on left object'
         );
-
-        // @todo test "left" priority flag
-        // @todo test includeRelations flag
-        // @todo test includeRelations in combination with overwriteWithEmpty
-        // @todo test has_one relations
-        // @todo test has_many and many_many relations
     }
 
     public function testPopulateDefaults()
@@ -1919,7 +1904,6 @@ class DataObjectTest extends SapphireTest
     public function testManyManyUnlimitedRowCount()
     {
         $player = $this->objFromFixture(DataObjectTest\Player::class, 'player2');
-        // TODO: What's going on here?
         $this->assertEquals(2, $player->Teams()->dataQuery()->query()->unlimitedRowCount());
     }
 

tests/php/ORM/DataQueryTest.php

@@ -202,9 +202,6 @@ class DataQueryTest extends SapphireTest
         );
     }
 
-    /**
-     * @todo Test paramaterised
-     */
     public function testNestedGroups()
     {
         $dq = new DataQuery(DataQueryTest\ObjectA::class);

tests/php/ORM/ManyManyThroughListTest/PolyItem.php

@@ -21,7 +21,6 @@ class PolyItem extends DataObject implements TestOnly
     /**
      * Placeholder for missing belongs_many_many for polymorphic relation
      *
-     * @todo Make this work for belongs_many_many
      * @return Generator|DataObject[]
      */
     public function Objects()

tests/php/ORM/PolymorphicHasManyListTest.php

@@ -10,8 +10,6 @@ use SilverStripe\ORM\Tests\DataObjectTest\Team;
  * Tests the PolymorphicHasManyList class
  *
  * @see PolymorphicHasManyList
- *
- * @todo Complete
  */
 class PolymorphicHasManyListTest extends SapphireTest
 {

tests/php/Security/MemberCsvBulkLoaderTest.php

@@ -104,7 +104,6 @@ class MemberCsvBulkLoaderTest extends SapphireTest
         DataObject::flush_and_destroy_cache();
         $member = DataObject::get_by_id(Member::class, $memberID);
 
-        // TODO Direct getter doesn't work, wtf!
         $this->assertEquals(Security::config()->password_encryption_algorithm, $member->getField('PasswordEncryption'));
         $auth = new MemberAuthenticator();
         $result = $auth->checkPassword($member, 'mypassword');

tests/php/Security/SecurityDefaultAdminTest.php

@@ -21,8 +21,6 @@ class SecurityDefaultAdminTest extends SapphireTest
     {
         parent::setUp();
 
-        // TODO Workaround to force database clearing with no fixture present,
-        // and avoid sideeffects from other tests
         if (!static::$tempDB->isUsed()) {
             static::$tempDB->build();
         }

tests/php/View/RequirementsTest.php

@@ -18,10 +18,6 @@ use SilverStripe\Dev\Deprecation;
 use SilverStripe\View\SSViewer;
 use SilverStripe\View\ThemeResourceLoader;
 
-/**
- * @todo Test that order of combine_files() is correct
- * @todo Figure out how to clear the modified state of Requirements class - might affect other tests.
- */
 class RequirementsTest extends SapphireTest
 {
 

tests/php/View/SSViewerTest.php

@@ -2057,7 +2057,6 @@ EOC;
 
     public function testRequireCallInTemplateInclude()
     {
-        //TODO undo skip test on the event that templates ever obtain the ability to reference MODULE_DIR (or something to that effect)
         if (FRAMEWORK_DIR === 'framework') {
             $template = new SSViewer(['SSViewerTestProcess']);