[PP-1] Fold in dependency parsing wisdom from project_composer

Im going to move the parent issue to the new meta.

Folds in and includes #3047898: project_composer_parse_dependency() behaviour for <=8.x-1.x is not consistent with composer/semver

Remove left-over case from debugging

Looks great! Just the code sniffer issues from what I can see.

Fixes code sniff issues.

Looking good already, but might just be me, but the whole flow of the code is a little hard to follow right now. Little too many nested if's and switch stuff going on here in one big method.

So mostly some observations that might make this a little easier to follow.

1. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -98,8 +112,14 @@ public function isCompatible($version) {
   +   *   Constraint as array.
   

   as 'an' array?

2. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    $p_op = '(?P<operation><>|!=|==|<=|>=|=|<|>|~|\^)?';
   ...
   +    $p_core = '(?P<core>(?:\d+\.x-(?=\d))?)';
   +    $p_major = '(?P<major>\d+)?';
   ...
   +    $p_minor = '\.?(?P<minor>(?:\d+|x(?=\.|$|-dev))?)';
   +    $p_patch = '\.?(?P<patch>(?:\d+|x(?=\.|$))?)';
   +    $p_stability = '(?P<stability>(?:-[A-Za-z]+)?)';
   +    $p_stability_ver = '(?P<stability_ver>(?:\d+)?)';
   

   the $p_ prefix for these variables is unclear

3. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    // How many ops? 0, 1 or 2
   +    // Which op?
   +    // branch or version?
   +    // How many digits?
   +    // LSD is x?
   

   Can we turn this into a proper english paragraph?

4. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    // do we have any constraints at all?
   +    if ($this->constraint) {
   

   Can we turn this into an early return instead of one giant nested if(). And update the comment to reflect that?

5. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   ...
   +        // If stability is 'dev' we pretend we didnt see that.
   

   didnt => didn't

6. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +      if (count($constraints) == 1) {
   ...
   +        return $op . $matches['major'] . $matches['minor'] . $matches['patch'] . $matches['stability'] . $matches['stability_ver'];
   

   This if() always leads to the return at the end of it. Can we split this logic into a separate method and just make this if return the result of that? Might make this whole thing a little easier to read.

7. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +      // Two constraints - two operators.
   +      preg_match("/^\s*$p_op\s*$p_core$p_major$p_minor$p_patch$p_stability$p_stability_ver/", $constraints[0], $first_matches);
   

   The same as for the 'one constraint' logic, can this just be split into a separate method or something?

edit: fixed some dreditor messes I clicked myself into

+++ b/core/lib/Drupal/Component/Version/Constraint.php
@@ -130,6 +150,194 @@ private function parseConstraint($constraint_string, $core_compatibility) {
+    return '*';

Also, we seem to be missing test coverage for this?

Addresses #12 and #13 most of these are as found in project_composer, so tidied up a bit and refactored to get rid of else/elseif - it feels cleaner.

Also made the method public as I found I need it in the BC layer in #3005229: Provide optional support for using composer.json for dependency metadata

@larowlan wrote an amazing patch, over in #3005229: Provide optional support for using composer.json for dependency metadata, to make composer.json the canonical source of module metadata.🤓😲It includes the code from this issue, so this is a blocker for that one.

Brief, shallow review. The actual logic is way too complicated for me to parse, especially this close to bedtime.

1. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -14,6 +16,20 @@ class Constraint {
   +   * Core compatability string.
   

   Misspelled; should be "compatibility". Also, can the doc comment provide an example of what the value of this property would be?

2. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -14,6 +16,20 @@ class Constraint {
   +  /**
   +   * Constraint as composer string.
   

   Nit: "composer" should be "Composer". This doc comment would also benefit from an example, I think.

3. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -82,12 +98,10 @@ public function toArray() {
   +    if (!$this->constraint || empty($version)) {
   +      return TRUE;
   

   This could use a comment.

4. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -98,18 +112,24 @@ public function isCompatible($version) {
   +   * @return array[]
   +   *   Constraint as an array.
   

   Can this comment expand on what the return value might look like?

5. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -98,18 +112,24 @@ public function isCompatible($version) {
   -    $p_op = '(?<operation>!=|==|=|<|<=|>|>=|<>)?';
   +    $op_pattern = '(?<operation>!=|==|=|<|<=|>|>=|<>)?';
   

   Thank you for renaming this variable. :)

6. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -98,18 +112,24 @@ public function isCompatible($version) {
   -    $p_minor = '(?<minor>(?:\d+|x)(?:-[A-Za-z]+\d+)?)';
   +    $minor_pattern = '(?<minor>(?:\d+|x)(?:-[A-Za-z]+\d+)?)';
   

   And this one!

7. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +  /**
   +   * Gets constraint in a composer compatible format.
   

   Nit: Should be "Composer-compatible".

8. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +   *   The constraint expressed in a format suitable for use with composer.
   

   As elsewhere, Composer is a proper name, so it should be capitalized. I'll stop mentioning it now. :)

9. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    // We use named subpatterns and support every op that version_compare
   

   "op" seems to suggest "operation", as opposed to "operator". So we should probably just say "operator".

10. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +    // do we have any constraints at all?
    +    if (!$this->constraint) {
    +      return '*';
    +    }
    

    This comment can be improved. :)

11. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +      preg_match("/^\s*$op_pattern\s*$core_pattern$major_pattern$minor_pattern$patch_pattern$stability_pattern$stability_version_pattern/", trim($constraints[0]), $matches);
    

    This expression looks like it's repeated three times in the same method. Maybe it should be a variable?

12. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +   * @return string
    +   *   Parsed single constraing.
    

    "constraint" :)

13. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +    if (!empty($matches['stability']) && $matches['stability'] == '-dev') {
    

    We should use === here.

14. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +        if ($matches['minor'] == 'x') {
    +          $matches['minor'] = '0';
    +          return $this->buildComposerConstraint($matches, $op);
    +        }
    +        if ($matches['patch'] == 'x' || $matches['patch'] == '') {
    

    Also here.

15. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +      if ($matches['minor'] == 'x') {
    +        $matches['minor'] = '';
    +      }
    +      if ($matches['patch'] == 'x') {
    +        $matches['patch'] = '';
    +      }
    

    And here, as well as the rest of the method.

16. +++ b/core/lib/Drupal/Component/Version/Constraint.php
    @@ -130,6 +150,213 @@ private function parseConstraint($constraint_string, $core_compatibility) {
    +        if (($matches['minor'] == '') || ($matches['minor'] == 'x')) {
    

    There appear to be superfluous parentheses here.

I see one thing missing from the test coverage: I feel like I have seen constraints as ^2@beta or ^2@alpha.

Found some examples in a project:

"drupal/search_api_location": "^1.0@alpha",
"drupal/path_redirect_import": "^1.0@beta",
"drupal/dropzonejs_eb_widget": "^2.0@alpha",
        "drupal/focal_point": "^1.0@beta",
        "drupal/geocoder": "^2.0@beta",
        "drupal/geofield": "^1.0@beta",

We didn't mark them this way, Composer did after composer require. This is the root project's composer.json and something Drupal won't grok. But I'm assuming these should be added to the test coverage.

1. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -104,4 +108,93 @@ public function testToArray() {
   +      'no constraint' => ['', '*'],
   ...
   +      'No constraint' => ['', '*'],
   

   Duplicates

2. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -98,18 +112,24 @@ public function isCompatible($version) {
   -    $p_core = '(?:' . preg_quote($core_compatibility) . '-)?';
   

   Because there's no corollary of this in the composer code I think this might lead to interesting abilities for the composer driven isCompatible().

   So if I add

       // Test greater than and less than with an incorrect core compatbility.
       $less_and_greater = new Constraint('<8.x-4.x,>8.x-1.x', '7.x');
       $tests['(<8.x-4.x,>8.x-1.x)-4.0-7.x'] = [$less_and_greater, '4.0', FALSE];
       $tests['(<8.x-4.x,>8.x-1.x)-3.9-7.x'] = [$less_and_greater, '3.9', FALSE];
       $tests['(<8.x-4.x,>8.x-1.x)-2.1-7.x'] = [$less_and_greater, '2.1', FALSE];
       $tests['(<8.x-4.x,>8.x-1.x)-1.9-7.x'] = [$less_and_greater, '1.9', FALSE];
   

   to \Drupal\Tests\Component\Version\ConstraintTest::providerIsCompatible() - HEAD passes... with this patch it fails. Also what is super interesting is last test fails with the patch and I expected only the middle two to fail... huh.

I see one thing missing from the test coverage: I feel like I have seen constraints as ^2@beta or ^2@alpha.

In info files? Because the current logic is taken from project_composer which is part of the d.o composer facade which translates info files to composer.json and was based on Mixologic querying all the stuff seen in existing modules

@larowlan no, not from info files, these are in composer.json. I assumed we needed Drupal core to grok the @STABILITY tags. Those values are returned from the Composer facade as far as I know.

@larowlan no, not from info files, these are in composer.json. I assumed we needed Drupal core to grok the @STABILITY tags. Those values are returned from the Composer facade as far as I know.

No its the other way here mate, we're providing the composer facade in core so that info files are cast to composer.json in a BC layer.

Fixes #18 and #16

No its the other way here mate, we're providing the composer facade in core so that info files are cast to composer.json in a BC layer.

😄cool, I just wanted to check

+++ b/core/lib/Drupal/Component/Version/Constraint.php
@@ -130,6 +154,214 @@ private function parseConstraint($constraint_string, $core_compatibility) {
+  private function buildComposerConstraint(array $matches, string $op): string {
...
+  private function processDualConstraintsMatches($first_matches, $second_matches): string {

Are we doing return type hinting now? And if so, why just these two methods?

Phpstorm is too smart for it's own good, removed those typehints

@larowlan thanks! Looks great to me now, all feedback has been addressed.

1. +++ b/core/modules/system/tests/src/Functional/Module/VersionTest.php
   @@ -30,7 +30,7 @@ public function testModuleVersions() {
   -      'common_test (=8.x2.x, >=2.4)',
   +      'common_test (=8.x2.x)',
   

   So if I add back 'common_test (=8.x2.x, >=2.4)' - is that really correct? It seems we've changed the meaning of (=8.x2.x, >=2.4) from having to satisfy both constraints to be being an OR. Not sure that's right.

2. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -104,4 +115,92 @@ public function testToArray() {
   +      // Broken nonsense.
   +      '=8.x2.x, >=2.4' => ['=8.x2.x, >=2.4', '=8 || >=2.4'],
   

   This doesn't look very broken.

3. One thing that surprises me about the patch is the relative simplicity of \Drupal\Component\Version\Constraint::processDualConstraintsMatches() vs the relative complexity in \Drupal\Component\Version\Constraint::processSingleConstraintMatches() - why's this? I would have thought that a single constraint match was less complex than a dual?

1. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +154,214 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +  public function getComposerConstraint() {
   

   I see that @larowlan made this method public in #14

   but since that issue is postponed on 2 other issue couldn't we just make the method public then? This class is marked as internal so it seems like we are creating an API we don't need too. What if in that issue we want to refactor and ComposerHelper::convertFromDrupalConstraintFormat() as so don't need the current method at all. We couldn't remove it because it is public.

2. re #26.1 I don't think the patch actually needs this change. It seems like we should be changing existing tests cases but only adding new ones. If we have to change an existing test case to make the test pass then we have a BC problem.

   I removed this change and the test still pass so the comma is still enforcing an AND. I don't think we need any changes to VersionTest in this issue.

3. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -81,6 +81,10 @@ public function providerIsCompatible() {
   +    // Test greater than and equals than.
   +    $greater_and_equal = new Constraint('=8.x-2.x, >=2.4-alpha2', '8.x');
   +    $tests['(=8.x-2.x, >=2.4-alpha2)-8.x-2.4-beta3'] = [$greater_and_equal, '2.4-beta3', TRUE];
   +
   
   @@ -88,6 +92,13 @@ public function providerIsCompatible() {
   +    // Test greater than and less than with an incorrect core compatbility.
   +    $less_and_greater = new Constraint('<8.x-4.x,>8.x-1.x', '7.x');
   +    $tests['(<8.x-4.x,>8.x-1.x)-4.0-7.x'] = [$less_and_greater, '4.0', FALSE];
   +    $tests['(<8.x-4.x,>8.x-1.x)-3.9-7.x'] = [$less_and_greater, '3.9', FALSE];
   +    $tests['(<8.x-4.x,>8.x-1.x)-2.1-7.x'] = [$less_and_greater, '2.1', FALSE];
   +    $tests['(<8.x-4.x,>8.x-1.x)-1.9-7.x'] = [$less_and_greater, '1.9', FALSE];
   +
   

   This all points to a lack of test coverage for the current functionality.

   All the new cases pass with any changes to \Drupal\Component\Version\Constraint in this issue.

   It seems weird to adding new test coverage for existing functionality and changing how that same functionality works under the hood in the same issue. Right now I don't think this issue has any test-only patches so the only way we can be sure the new test cases aren't actually showing new/changed functionality is if people have been running these locally without the Constraint changes.

   I am uploading a test-only patch.

1. Adding #2641658: Module version dependency in .info.yml is ineffective for patch releases as related because these changes would actually fix the problem.
   Adding those test cases to this patch
2. Also removing the chagne to VersionTest.php as per #28.2
3. This issue reminds me of #2990511: Add comprehensive test coverage for update status for security releases where we split out the actually test coverage that just covered the current functionality to it's own issue.

   Would that be good idea because I feel like \Drupal\Tests\Component\Version\ConstraintTest::providerIsCompatible() is not very comprehensive right now. We are already adding test coverage for existing functionality in this issue. see #28.3

   Also since this issue does change functionality in that it fixes #2641658 are we sure it doesn't change other functionality that we don't want? If had better test coverage it would seem safer.

1. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +154,214 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    if ($first_matches['operation'] === '=' || $second_matches['operation'] === '=') {
   +      // Providing an explicit version (=) and a second constraint implies an
   +      // OR.
   +      $separator = ' || ';
   +    }
   

   This is the problem.

   This is not how the current behavior works although it seem logical. Even with an "=" it uses AND.

2. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -81,6 +81,10 @@ public function providerIsCompatible() {
   +    // Test greater than and equals than.
   +    $greater_and_equal = new Constraint('=8.x-2.x, >=2.4-alpha2', '8.x');
   +    $tests['(=8.x-2.x, >=2.4-alpha2)-8.x-2.4-beta3'] = [$greater_and_equal, '2.4-beta3', TRUE];
   

   This the only test case that has an "=" and multiple constraints.

   The reason that this passes in the test-only patch in #28 is because 2.4-beta3 passes for both constraints so it works whether an AND or OR operator is used.

   I will add a test case for this.

3. +++ b/core/lib/Drupal/Component/Version/Constraint.php
   @@ -130,6 +154,214 @@ private function parseConstraint($constraint_string, $core_compatibility) {
   +    if (count($constraints) == 1) {
   +      // One version constraint. - zero or one operators
   +      preg_match($pattern, trim($constraints[0]), $matches);
   +      return $this->processSingleConstraintMatches($matches);
   +    }
   +    // Two constraints - two operators.
   +    preg_match($pattern, $constraints[0], $first_matches);
   +    preg_match($pattern, $constraints[1], $second_matches);
   +    return $this->processDualConstraintsMatches($first_matches, $second_matches);
   

   This assumes only 1 or 2 constraints but currently we support more than 2, always with AND.

   We even document this https://www.drupal.org/docs/7/creating-custom-modules/writing-module-inf...
   dependencies[] = exampleapi (>1.0, <=3.2, !=3.0)
   (I tested and works on 8.x too)

   I will add a test to prove this fails and pass with changes.

4. I am removing testGetComposerConstraint() so we can patches with exact changes to ConstraintTest with and without changes to Constraint this will show 3 fails with changes.

   I think we should make test only issue to harden are existing tests for this

I opened #3066448: Harden test coverage for dependency Constraint class

In addition the problems mentioned in #31 that also shows that we currently support '<>' for not equals which this patch does not.

I am uploading at patch with the changes to \Drupal\Component\Version\Constraint from this issue and the changes to \Drupal\Tests\Component\Version\ConstraintTest from the other issue

Will set to Needs Review but then I think should postpone this issue. Hopefully we can get the other issue committed soonish since it is only test changes.

whoops forgot to rebase my branch 😢

So there were 30 test failures plus some errors. Not as bad as seems because a lot of the test cases fail for the same reasons. Still think we should postpone and finish #3066448: Harden test coverage for dependency Constraint class so we can make this change with confidence that we aren't breaking current sites.

It seems like there are major risks for getting this wrong. The one that comes to mind is deployment scripts that enable modules that all of sudden start to not enable modules or enable modules that previously weren't being enabled by the scripts(because they didn't meet dependency requirements before this change but were left in scripts)

Moved this into the issue summary, because it's worth drawing attention to:

It seems like there are major risks for getting this wrong. The one that comes to mind is deployment scripts that enable modules that all of sudden start to not enable modules or enable modules that previously weren't being enabled by the scripts(because they didn't meet dependency requirements before this change but were left in scripts)

Also, raising priority to Major based on it.

• #26.3++ — same complexity observation here
• #28.1 ❤️ for thinking about API surface
• #28.2++ — removing/changing test cases implies backwards compatibility breaks. I agree with the related suggestion in #29.3.
• #28.3 + #31.2 + #33's <>: Wow, excellent finds!
• #31.1: this definitely fixed #26.1 now 👍

All caught up now. @tedbow already fixed most things. And he extracted the test coverage expansion/hardening to a separate issue to ensure this issue only is concerned with what's described in the issue title.

AFAICT the primary thing to still figure out — after #3066448: Harden test coverage for dependency Constraint class lands — is #26.3.

Got thinking about another potential problem with this approach.

Would \Drupal\Component\Version\Constraint::getComposerConstraint() actually just pass a composer version string on without changing it? In some cases, yes

I think this introduces a problem because then it just passes it to Semver::satisfies(). I think some new to drupal devs would assume our constraints work with the composer format in .info.yml files and try it. If they happen to try strings that work then they would think this is the way it is suppose to work.

I think this will be more complicate if we do #3005229: Provide optional support for using composer.json for dependency metadata also(I think you could do that issue without this one) because I think devs would more likely think you can use composer strings in our info file.

So if we start to have devs using composer strings in the info file then the upgrade proposed in #3005229's UpgradeInfoFilesCommand.php becomes more complicate. Right now it assumes it is always drupal format. Also \Drupal\Component\Version\Constraint::getComposerConstraint() actually transform some composer strings and not others.

Here is test to prove the problem. It shows how composer string pass tests. interdiff against #24

1. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -99,6 +99,28 @@ public function providerIsCompatible() {
   +    $composer_tilde_constraint = new Constraint('~2.4.2', '8.x');
   +    $tests['(~2.4.2)-2.3'] = [$composer_tilde_constraint, '2.3', FALSE];
   +    $tests['(~2.4.2)-2.4.4'] = [$composer_tilde_constraint, '2.4.4', TRUE];
   +    $tests['(~2.4.2)-2.5'] = [$composer_tilde_constraint, '2.5', FALSE];
   +    $tests['(~2.4.2)-3.0'] = [$composer_tilde_constraint, '3.0', FALSE];
   

   So here is example of using a composer version constraint. I think people will start to do this.

2. +++ b/core/tests/Drupal/Tests/Component/Version/ConstraintTest.php
   @@ -200,6 +222,10 @@ public function providerGetComposerConstraint() {
   +      // @todo certain composer strings pass unaltered.
   +      '~~1.1.1' => ['~1.1.1', '~1.1.1'],
   +      '^2.4' => ['^2.4', '^2.4'],
   +      '2.4.*' => ['2.4.*', '~2.4.0'],
   

   Do we actually want this to happen? In some cases you could use composer directly and this could be very confusing because I think not in all cases.

   At least we don't have test coverage for it.

setting as needs work to figure that out but also it should then be postponed on #3066448: Harden test coverage for dependency Constraint class

Also needs a issue summary update because there is not state reason why we are doing this and what the benefit will be.

#3005229: Provide optional support for using composer.json for dependency metadata is postponed on this issue(UPDATE fixed my type) but I am not sure it needs to be.

#40: isn't this contradicting what #2313917-92: Core version key in module's .info.yml doesn't respect core semantic versioning is doing? Then again, #40 is 7 days old, #2313917-92 is 1 day old. Does this mean you've changed your mind about #40?

@Wim Leers re #42

No, right now approach in #2313917-92: Core version key in module's .info.yml doesn't respect core semantic versioning only concerns the core: key in .info.yml files. It uses \Composer\Semver\Semver::satisfies pretty much directly so it won't support the Drupal constraint strings(except for any that would be the exact same as strings that \Composer\Semver\Semver::satisfies would understand)

This current issue from my understanding only deals with the \Drupal\Component\Version\Constraint class which only affects the dependencies: key in .info.yml files. It doesn't actually intentionally make a change so that you can use composer strings in dependencies:. My point in #40 and #41 is that the way \Drupal\Component\Version\Constraint::parseToComposer() currently works it seems will effectively let you start to use some but not all composer strings.

If in #42 contradicting meant that if approach in #2313917-92: Core version key in module's .info.yml doesn't respect core semantic versioning was taken the core: key would support Composer dependency format and dependencies: would support our current Drupal specific format then yes. But we could discuss that on #2313917

I asked for an Issue Summary update in #41 because I don't think it is clear why we are doing this change and what the benefit is.

#3005229: Provide optional support for using composer.json for dependency metadata is postponed on this issue but I am not sure needs to be.(I mistyped in the last line of #41 fixed). see my comment which I will clarify after this.

Also #3005229 is tagged on Needs release manager review, Needs framework manager review. It would be pretty big change meaning for Drupal 9 or a release that supported both 8 & 9 you would need a composer.json file. It #3005229 gets rejected by those reviews do we still want/need this current change. If so again I think it should be clear why.

Closing as won't fix
See related plan #3069795-14: [meta] Improve dependency management for extensions as laid out by @Mixologic after discussion with @catch/@xjm/@larowlan/@Gábor Hojtsy/@tedbow/@Mixologic

tldr

1. #2313917: Core version key in module's .info.yml doesn't respect core semantic versioning allows us to not have to worry about semantic version constraints for the current usage - drupal:system (>=8.6.3) by instead using the core: ^8.6.3
2. #3005229: Provide optional support for using composer.json for dependency metadata will allow semantic version constraints by using composer.json
3. If we can avoid changing are existing logic in Constraints that is much less risky