encoding/wkt: preserve EMPTY members when unmarshalling a collection - #178
Open
gaoflow wants to merge 1 commit into
Open
encoding/wkt: preserve EMPTY members when unmarshalling a collection#178gaoflow wants to merge 1 commit into
gaoflow wants to merge 1 commit into
Conversation
Marshal emits spec-valid WKT for a GEOMETRYCOLLECTION whose members are
EMPTY geometries (e.g. GEOMETRYCOLLECTION(LINESTRING EMPTY)), but Unmarshal
silently dropped every empty member, so Unmarshal(Marshal(gc)) != gc.
splitGeometryCollection only yielded a member after seeing a '(' on its
stack; an EMPTY member has no parentheses, so it was never emitted, and
unmarshalCollection then discarded the "" result via len(g)==0. Rewrite the
splitter to split on top-level commas while tracking bracket depth, which
handles empty members and nested collections uniformly, and drop the swallow.
Add round-trip coverage over every empty-capable type, mixed empty/non-empty
and nested collections, plus direct splitter cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
wkt.Marshalproduces spec-valid WKT for aGEOMETRYCOLLECTIONwhose members are EMPTY geometries, butwkt.Unmarshalsilently drops those members, so the library cannot read back its own output:This reproduces for every empty-capable member type (
LINESTRING/POLYGON/MULTIPOINT/MULTILINESTRING/MULTIPOLYGON/GEOMETRYCOLLECTION EMPTY), for mixed empty/non-empty collections, and for nested collections.Root cause
splitGeometryCollectiononly appended a member once a(had been pushed onto its rune stack. An EMPTY geometry has no parentheses, so its token never triggered a yield and stayed stuck in the stack.unmarshalCollectionthen discarded the resulting""viaif len(g) == 0 { continue }.For example
splitGeometryCollection("(LINESTRING EMPTY)")returned[""]and"(POINT(1 2),LINESTRING EMPTY)"returned["", "POINT(1 2)"]-- the empty member vanished.Fix
Rewrite the splitter to split the collection body on top-level commas while tracking bracket depth. This treats empty members (no parens) and nested-collection members (inner commas) uniformly, and removes the
len(g)==0swallow. Non-empty and nested non-empty collections are unaffected.Tests
TestUnmarshalCollection_roundTripasserts theUnmarshal(Marshal(gc)) == gcself-oracle over each empty-capable type, mixed empty/non-empty, and nested collections, checking member count, member equality and marshal idempotence.TestSplitGeometryCollectioncovers the splitter directly, including nested members whose inner commas must not split.go test ./...passes for the affected packages (the pre-existingsimplifyVisvalingam benchmark failure is unrelated and also fails onmaster).