Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 43 additions & 11 deletions fragmentation/fragment.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ type Fragment struct {
Partitions []Partition
}

// partitionText contains selected source lines and their indentation group.
type partitionText struct {
// lines contains the source lines selected by one partition.
lines []string

// indentGroup identifies partitions whose common indentation is normalized together.
indentGroup string
}

// CreateDefaultFragment creates a whole-file fragment.
//
// Returns whole-file fragment.
Expand Down Expand Up @@ -76,15 +85,12 @@ func (f Fragment) text(lines []string, separator string) (string, error) {
if err != nil {
return "", err
}
var fragmentText []string
for _, partition := range partitionsTexts {
fragmentText = append(fragmentText, partition...)
}
indentation := indent.MaxCommonIndentation(fragmentText)
indentations := commonIndentations(partitionsTexts)

text := ""
for index, partitionText := range partitionsTexts {
cutIndentLines := indent.CutIndent(partitionText, indentation)
indentation := indentations[partitionText.indentGroup]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop variable partitionText on line 91 now shadows the partitionText type introduced at fragment.go:48, so the type name is unusable for the rest of the loop body and the reader meets partitionText.indentGroup (a value) a few lines after partitionText{...} (a type). Renaming the variable to partition, as commonIndentations already does, removes the collision.

cutIndentLines := indent.CutIndent(partitionText.lines, indentation)

if index > 0 {
separatorIndentation := separatorIndent(cutIndentLines)
Expand All @@ -105,17 +111,43 @@ func (f Fragment) text(lines []string, separator string) (string, error) {
// Returns:
// [][]string - selected lines grouped by partition.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale after the signature change: the function now returns []partitionText, not [][]string, and each element carries the indentation group as well as the lines.

// error - when a partition cannot select its lines.
func (f Fragment) obtainPartitionTexts(lines []string) ([][]string, error) {
var partitionLines [][]string
func (f Fragment) obtainPartitionTexts(lines []string) ([]partitionText, error) {
var partitions []partitionText
for _, part := range f.Partitions {
partitionText, err := part.Select(lines)
selectedLines, err := part.Select(lines)
if err != nil {
return nil, err
}
partitionLines = append(partitionLines, partitionText)
partitions = append(partitions, partitionText{
lines: selectedLines,
indentGroup: part.IndentGroup,
})
}

return partitions, nil
}

// commonIndentations calculates common indentation for every partition group.
//
// Parameters:
// partitions - provides selected source lines in source order.
//
// Returns indentation width by group name.
func commonIndentations(partitions []partitionText) map[string]int {
groupLines := make(map[string][]string)
for _, partition := range partitions {
groupLines[partition.indentGroup] = append(
groupLines[partition.indentGroup],
partition.lines...,
)
}

indentations := make(map[string]int, len(groupLines))
for indentGroup, lines := range groupLines {
indentations[indentGroup] = indent.MaxCommonIndentation(lines)
}

return partitionLines, nil
return indentations
}

// separatorIndent returns the indentation to use before a partition separator.
Expand Down
12 changes: 12 additions & 0 deletions fragmentation/fragment_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ type FragmentBuilder struct {
//
// Returns an error when the previous partition is still open.
func (b *FragmentBuilder) AddStartPosition(startPosition int) error {
return b.addStartPosition(startPosition, "")
}

// addStartPosition adds a partition with its indentation group.
//
// Parameters:
// startPosition - provides the zero-based source line where the partition starts.
// indentGroup - identifies partitions whose common indentation is normalized together.
//
// Returns an error when the previous partition is still open.
func (b *FragmentBuilder) addStartPosition(startPosition int, indentGroup string) error {
if !b.isPartitionsEmpty() {
lastPartition := b.lastAddedPartition()
if lastPartition.EndPosition < 0 {
Expand All @@ -60,6 +71,7 @@ func (b *FragmentBuilder) AddStartPosition(startPosition int) error {

partition := NewPartition()
partition.StartPosition = startPosition
partition.IndentGroup = indentGroup
b.Partitions = append(b.Partitions, partition)

return nil
Expand Down
15 changes: 9 additions & 6 deletions fragmentation/fragmentation.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ func (f Fragmentation) DoFragmentation() ([]string, map[string]Fragment, error)
func (f Fragmentation) parseLine(line string, contentToRender []string) ([]string, error) {
cursor := len(contentToRender)

docFragments, startErr := FindDocFragments(line)
docFragment, startErr := findDocFragmentDeclaration(line)
if startErr != nil {
return nil, startErr
}
Expand All @@ -159,8 +159,8 @@ func (f Fragmentation) parseLine(line string, contentToRender []string) ([]strin
}

switch {
case len(docFragments) > 0:
if err := f.parseStartDocFragments(docFragments, cursor); err != nil {
case len(docFragment.names) > 0:
if err := f.parseStartDocFragments(docFragment, cursor); err != nil {
return nil, err
}
case len(endDocFragments) > 0:
Expand All @@ -177,8 +177,11 @@ func (f Fragmentation) parseLine(line string, contentToRender []string) ([]strin
// parseStartDocFragments starts a new partition for each named fragment marker.
//
// It creates fragment builders when necessary.
func (f Fragmentation) parseStartDocFragments(docFragments []string, cursor int) error {
for _, fragmentName := range docFragments {
func (f Fragmentation) parseStartDocFragments(
declaration fragmentDeclaration,
cursor int,
) error {
for _, fragmentName := range declaration.names {
fragment, exists := f.fragmentBuilders[fragmentName]
if !exists {
builder := FragmentBuilder{
Expand All @@ -188,7 +191,7 @@ func (f Fragmentation) parseStartDocFragments(docFragments []string, cursor int)
f.fragmentBuilders[fragmentName] = &builder
fragment = f.fragmentBuilders[fragmentName]
}
if err := fragment.AddStartPosition(cursor); err != nil {
if err := fragment.addStartPosition(cursor, declaration.indentGroup); err != nil {
return err
}
}
Expand Down
143 changes: 141 additions & 2 deletions fragmentation/fragmentation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const (
twoFragmentsFileName = "TwoFragments.java"
overlappingFragmentsFileName = "OverlappingFragments.java"
emptyLaterPartitionsFileName = "EmptyLaterPartitions.java"
groupedIndentFileName = "GroupedIndent.java"
emptyFileName = "Empty.java"
indent = " "
)
Expand Down Expand Up @@ -430,7 +431,11 @@ var _ = Describe("Fragmentation", func() {
It("should report malformed fragment markers with source line context", func() {
sourceRoot := GinkgoT().TempDir()
sourcePath := filepath.Join(sourceRoot, "Malformed.java")
Expect(os.WriteFile(sourcePath, []byte("// #docfragment"), 0600)).To(Succeed())
Expect(os.WriteFile(
sourcePath,
[]byte(`// #docfragment "main" indentgroup="imports"`),
0600,
)).To(Succeed())
frag, err := fragmentation.NewFragmentation(sourcePath)
Expect(err).ShouldNot(HaveOccurred())

Expand All @@ -442,7 +447,7 @@ var _ = Describe("Fragmentation", func() {
ContainSubstring("failed to do fragmentation"),
ContainSubstring("file://"),
ContainSubstring("Malformed.java:1"),
ContainSubstring("without any name"),
ContainSubstring("unexpected text after `#docfragment` declaration"),
)))
})

Expand Down Expand Up @@ -619,6 +624,33 @@ line
Expect(openings[1]).Should(Equal(subMainFragment))
})

It("should find fragment openings with an indentation group", func() {
docFragment := fmt.Sprintf(
"// #docfragment \"%s\",\"%s\" indent-group=\"imports\"",
mainFragment,
subMainFragment,
)

openings, err := fragmentation.FindDocFragments(docFragment)

Expect(err).ShouldNot(HaveOccurred())
Expect(openings).Should(Equal([]string{mainFragment, subMainFragment}))
})

It("should allow fragment markers inside block comments", func() {
openings, startErr := fragmentation.FindDocFragments(
`<!-- #docfragment "main" indent-group="imports" -->`,
)
endings, endErr := fragmentation.FindEndDocFragments(
`/* #enddocfragment "main" */`,
)

Expect(startErr).ShouldNot(HaveOccurred())
Expect(openings).Should(Equal([]string{mainFragment}))
Expect(endErr).ShouldNot(HaveOccurred())
Expect(endings).Should(Equal([]string{mainFragment}))
})

It("should correctly find fragment endings", func() {
endDocFragment := fmt.Sprintf(
"// #enddocfragment \"%s\",\"%s\"", mainFragment, subMainFragment)
Expand Down Expand Up @@ -663,6 +695,113 @@ line
ContainSubstring("invalid syntax"),
)))
})

It("should report an unquoted fragment name before trailing text", func() {
openings, err := fragmentation.FindDocFragments("// #docfragment main trailing")

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError(And(
ContainSubstring("failed to unquote name `main`"),
ContainSubstring("invalid syntax"),
)))
})

It("should allow escaped quotes in fragment names", func() {
openings, err := fragmentation.FindDocFragments(
`// #docfragment "main\"part"`,
)

Expect(err).ShouldNot(HaveOccurred())
Expect(openings).Should(Equal([]string{`main"part`}))
})

It("should report an indentation group without an equals sign", func() {
openings, err := fragmentation.FindDocFragments(
`// #docfragment "main" indent-group "imports"`,
)

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError(
`indent-group must use the form indent-group="name"`,
))
})

It("should report an unquoted indentation group", func() {
openings, err := fragmentation.FindDocFragments(
"// #docfragment \"main\" indent-group=imports",
)

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError(ContainSubstring(
"indent-group value `imports` must be quoted",
)))
})

It("should report an empty indentation group", func() {
openings, err := fragmentation.FindDocFragments(
"// #docfragment \"main\" indent-group=\"\"",
)

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError("indent-group must not be empty"))
})

It("should report an unterminated indentation group", func() {
openings, err := fragmentation.FindDocFragments(
`// #docfragment "main" indent-group="imports`,
)

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError(And(
ContainSubstring(`failed to unquote indent-group `),
ContainSubstring("invalid syntax"),
)))
})

It("should reject an indentation group on an end marker", func() {
endings, err := fragmentation.FindEndDocFragments(
"// #enddocfragment \"main\" indent-group=\"imports\"",
)

Expect(endings).Should(BeEmpty())
Expect(err).Should(MatchError(ContainSubstring(
"indent-group is only supported by #docfragment",
)))
})

It("should reject unrecognized text after a fragment declaration", func() {
invalidSuffixes := []string{
`indentgroup="imports"`,
`indent_group="imports"`,
`INDENT-GROUP="imports"`,
`indent-group="a" indent-group="b"`,
}
for _, suffix := range invalidSuffixes {
openings, err := fragmentation.FindDocFragments(
`// #docfragment "main" ` + suffix,
)

Expect(openings).Should(BeEmpty())
Expect(err).Should(MatchError(ContainSubstring(
"unexpected text after `#docfragment` declaration",
)), suffix)
}
})
})

It("should normalize common indentation within each indentation group", func() {
content := resolveTestFragment(resolver, groupedIndentFileName, "Example", config)

Expect(content).Should(Equal([]string{
"import java.util.List;",
config.Separator,
"var first = values.get(0);",
indent + "var nested = first.trim();",
config.Separator,
"var second = values.get(1);",
config.Separator,
"System.out.println(nested + second);",
}))
})

It("should render empty later partitions with an unindented separator", func() {
Expand Down
Loading
Loading