[spec] Nesting cat inside ? causes unform to return nested result

Description

Calling conform and then unform with a spec that consists of some cat nested inside of some ? creates an extra level of nesting in the result:

This behavior does not occur with just ? or cat alone:

Patch: CLJ-2003-corrected.patch

Environment

None

Activity

Show:
Tyler Tallman
August 10, 2017, 3:41 AM

This seems to be all that is needed.

Francis Avila
August 29, 2017, 3:24 AM

The problem is actually more universal than (? (cat ...)). s/unform of a s/? with any regex child op will introduce an extra level of nesting. When the child is a regex, we are consuming the same "level" of sequence so unform should not introduce an extra level. However in other cases (non-regex ops), we should still possibly produce a nested collection.

The previous patch was too aggressive: it unwrapped all sub-unforms of s/?. This patch CLJ-2003-corrected.patch only unwraps when the sub-op is a regex.

Unfortunately it is impossible to distinguish between a desired-but-optional nil and a non-match from s/?. Specifically, the following tests now hold:

(I did not add these tests to the patch because I was unsure if they should be part of the contract of unform. However, they are pretty big gotchas.)

I also added tests for every possible subop of s/?, except ::s/accept, which I could not think of a test case for. (I'm not sure ::s/accept is actually reachable inside s/op-unform?)

Alex Miller
August 29, 2017, 1:33 PM

Thanks for working on this - I will take a look when I get a chance.

Francis Avila
August 29, 2017, 3:20 PM

I had to amend my patch slightly (same name): one of the test cases wasn't testing the correct thing.

Francis Avila
July 31, 2018, 7:12 PM

Patch was no longer applying cleanly to master because of tests added by other commits. Patch rebased to master

Assignee

Unassigned

Reporter

import

Labels

Approval

Vetted

Patch

Code and Test

Affects versions

Priority

Critical