As outlined in the related issues, some Rules Context code was moved to typed_data more than three years ago by commit a98ce54. There is no issue associated with this commit.
Specifically, four classes were copied:
src/Context/ContextDefinition.php
src/Context/ContextDefinitionInterface.php
src/Context/AnnotatedClassDiscovery.php
src/Context/Annotation/ContextDefinition.php
To this day these remain almost-identical copies of the Rules code, with the exception of src/Context/ContextDefinition.php which has diverged for D9 because of #3161582: EntityContextDefinition breaks the context system.
It is troublesome that the typed_data version of these classes don't seem to be used by typed_data, as evidenced by the related issues - both #3168398: Don't use Doctrine directly and #3161582: EntityContextDefinition breaks the context system should have caused typed_data tests to fail. So either we're not using the typed_data classes or our tests need some seriously improved coverage.
Likewise, much of the Context code remains in Rules. If we NEED it here in typed_data, then Rules should be using the typed_data version, not a copy in Rules. Perhaps the Context code should be split of into its own module, but NOT its own composer package as @fago suggested long ago in #2677098: [META] Provide Context related code as composer package. But certainly we need to examine this and remove the code duplication which can only cause problems in the future as the copies diverge over time.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
tr commentedWe now have a 2.0.x branch. My intention is to deprecate the use of Typed Data API's context code in the 8.x-1.x branch then remove it entirely in the 2.0.x.
Leaving this issue assigned to the 8.x-1.x branch because the deprecation has to be done there first, and present in a fixed point release (8.x-1.0) so that the API change this represents can be properly handled by other modules that use Typed Data API.
Comment #5
aaronbaumanMR6 adds deprecation notices to the Context classes for 1.x branch.
See followup issue for 2.0.x #3344074: Remove Context classes
Comment #6
aaronbaumanComment #7
tr commentedThanks. Can you fix up the @deprecated statements so they don't generate those coding standards messages?
Comment #8
tr commentedAlso, don't we need a @trigger_error on the classes?
Comment #9
aaronbaumanWould you want the
trigger_errorjust inside the class statement? Or within each method?I can also look into a deprecated core class and see what they did.
Comment #10
tr commentedI believe the rules for how to properly deprecate classes are in the coding standards document somewhere - I haven't looked at it recently, but in the changes I made locally (back in December) for testing I put a @trigger_error between the namespace and the first use on all the deprecated classes.
EDIT: https://www.drupal.org/about/core/policies/core-change-policies/drupal-d...
Comment #11
aaronbaumanUpdated annotation strings and added trigger_error()s
Comment #12
tr commentedWith this, there should be a blank line between the @deprecated tag and the @see tag.
Is this the proper way to deprecate Interfaces? I looked it up last year and didn't find any information or examples in core, which is why I never did this myself. Will deprecating an Interface like the above patch actually trigger deprecation notices in code that uses a class implementing that interface? The closest I could find to deprecating an Interface was to deprecate each of the methods individually. Does this need to be done, or can we get away with doing the same thing we do with classes?
Comment #14
tr commentedCommitted.