Improve error feedback

embedding/embedding_test.go

@@ -131,7 +131,9 @@ var _ = Describe("Embedding", func() {
 		Expect(err).Should(HaveOccurred())
 		Expect(err.Error()).Should(And(
 			ContainSubstring("missing-closing-tag.md"),
-			ContainSubstring("the `<embed-code>` tag is not closed"),
+			ContainSubstring(
+				"the `<embed-code>` instruction is not closed; add `</embed-code>` or use `/>`",
+			),
 			ContainSubstring("unclosed-nested-tag.md"),
 			ContainSubstring("element <unexpected> closed by </embed-code>"),
 		))
@@ -245,7 +247,7 @@ var _ = Describe("Embedding", func() {
 		Expect(err.Error()).Should(ContainSubstring(
 			"missing-closing-tag.md:3`: " +
 				"failed to parse an embedding instruction: " +
-				"the `<embed-code>` tag is not closed",
+				"the `<embed-code>` instruction is not closed; add `</embed-code>` or use `/>`",
 		))
 	})
 
@@ -264,7 +266,9 @@ var _ = Describe("Embedding", func() {
 		var parseErr parsing.InstructionParseError
 		Expect(errors.As(err, &parseErr)).Should(BeTrue())
 		Expect(parseErr.Line).Should(Equal(3))
-		Expect(parseErr.Reason).Should(Equal("the `<embed-code>` tag is not closed"))
+		Expect(parseErr.Reason).Should(Equal(
+			"the `<embed-code>` instruction is not closed; add `</embed-code>` or use `/>`",
+		))
 	})
 
 	It("should report the XML parser error", func() {
@@ -294,6 +298,28 @@ var _ = Describe("Embedding", func() {
 		))
 	})
 
+	It("should report a missing file attribute with documentation context", func() {
+		docPath := testDocPath(config, "missing-file-attribute.md")
+		Expect(os.WriteFile(
+			docPath,
+			[]byte("# Missing file attribute\n\n"+
+				"<embed-code fragment=\"main()\"/>\n"+
+				"```java\n"+
+				"```\n"),
+			0600,
+		)).To(Succeed())
+		processor := newProcessor(docPath, config)
+
+		_, err := processor.Embed()
+
+		Expect(err).Should(HaveOccurred())
+		Expect(err.Error()).Should(ContainSubstring(
+			"missing-file-attribute.md:3`: " +
+				"failed to parse an embedding instruction: " +
+				"<embed-code> must specify a non-empty `file` attribute",
+		))
+	})
+
 	It("should report an unclosed code fence after the instruction", func() {
 		docPath := testDocPath(config, "unclosed-code-fence.md")
 		processor := newProcessor(docPath, config)

embedding/parsing/instruction.go

@@ -29,6 +29,7 @@ package parsing
 import (
 	"fmt"
 	"log/slog"
+	"slices"
 	"strings"
 
 	"embed-code/embed-code-go/configuration"
@@ -37,6 +38,15 @@ import (
 	"embed-code/embed-code-go/indent"
 )
 
+var supportedInstructionAttributes = []string{
+	"comments",
+	"end",
+	"file",
+	"fragment",
+	"line",
+	"start",
+}
+
 // Instruction specifies the code fragment to embed into a Markdown file.
 //
 // It is parsed from an XML-like `<embed-code>` instruction such as
@@ -123,6 +133,10 @@ func (e PatternNotFoundError) Error() string {
 // error - when instruction attributes are invalid.
 func NewInstruction(
 	attributes map[string]string, config configuration.Configuration) (Instruction, error) {
+	if err := validateInstructionAttributes(attributes); err != nil {
+		return Instruction{}, err
+	}
+
 	codeFile := attributes["file"]
 	fragment := attributes["fragment"]
 	startValue := attributes["start"]
@@ -153,6 +167,35 @@ func NewInstruction(
 	}, nil
 }
 
+// validateInstructionAttributes reports unsupported or missing instruction attributes.
+func validateInstructionAttributes(attributes map[string]string) error {
+	var unsupported []string
+	for attribute := range attributes {
+		if !slices.Contains(supportedInstructionAttributes, attribute) {
+			unsupported = append(unsupported, attribute)
+		}
+	}
+	if len(unsupported) > 0 {
+		slices.Sort(unsupported)
+		attributeLabel := "attribute"
+		if len(unsupported) > 1 {
+			attributeLabel = "attributes"
+		}
+
+		return fmt.Errorf(
+			"unsupported <embed-code> %s `%s`; expected one of `%s`",
+			attributeLabel,
+			strings.Join(unsupported, "`, `"),
+			strings.Join(supportedInstructionAttributes, "`, `"),
+		)
+	}
+	if strings.TrimSpace(attributes["file"]) == "" {
+		return fmt.Errorf("<embed-code> must specify a non-empty `file` attribute")
+	}
+
+	return nil
+}
+
 // validateExclusiveAttributes reports mutually exclusive instruction attributes.
 func validateExclusiveAttributes(fragment string, start string, end string, line string) error {
 	if fragment != "" && (start != "" || end != "" || line != "") {

embedding/parsing/instruction_test.go

@@ -126,6 +126,51 @@ var _ = Describe("Instruction", func() {
 		Expect(parsing.FromXML(xmlString, config)).Error().Should(HaveOccurred())
 	})
 
+	It("should report a missing file attribute", func() {
+		xmlString := `<embed-code fragment="main()"/>`
+
+		_, err := parsing.FromXML(xmlString, config)
+
+		Expect(err).Should(MatchError(
+			"<embed-code> must specify a non-empty `file` attribute",
+		))
+	})
+
+	It("should report an unsupported instruction attribute", func() {
+		xmlString := `<embed-code fil="org/example/Hello.java"/>`
+
+		_, err := parsing.FromXML(xmlString, config)
+
+		Expect(err).Should(MatchError(
+			"unsupported <embed-code> attribute `fil`; " +
+				"expected one of `comments`, `end`, `file`, `fragment`, `line`, `start`",
+		))
+	})
+
+	It("should report unsupported instruction attributes", func() {
+		xmlString := `<embed-code ` +
+			`fil="org/example/Hello.java" ` +
+			`unknown="value" ` +
+			`file="org/example/Hello.java"/>`
+
+		_, err := parsing.FromXML(xmlString, config)
+
+		Expect(err).Should(MatchError(
+			"unsupported <embed-code> attributes `fil`, `unknown`; " +
+				"expected one of `comments`, `end`, `file`, `fragment`, `line`, `start`",
+		))
+	})
+
+	It("should report a mistyped instruction tag", func() {
+		xmlString := `<embed-codee file="org/example/Hello.java"/>`
+
+		_, err := parsing.FromXML(xmlString, config)
+
+		Expect(err).Should(MatchError(
+			"expected `<embed-code>` instruction tag, got `<embed-codee>`",
+		))
+	})
+
 	It("should have an error for an invalid glob pattern", func() {
 		instructionParams := TestInstructionParams{
 			startGlob: "[",

embedding/parsing/instruction_token.go

@@ -142,13 +142,22 @@ func (e EmbedInstructionTokenState) Accept(context *Context,
 
 // parseFailureReason explains why an embedding instruction could not be parsed.
 func parseFailureReason(instructionBody []string, parseErr error) string {
-	instruction := strings.TrimSpace(strings.Join(instructionBody, " "))
-	if !strings.Contains(instruction, "/>") &&
-		!strings.Contains(instruction, "</"+EmbeddingTag+">") {
-		return fmt.Sprintf("the `<%s>` tag is not closed",
+	var wrongTagErr WrongInstructionTagError
+	if errors.As(parseErr, &wrongTagErr) {
+		return wrongTagErr.Error()
+	}
+	if !openingTagClosed(instructionBody) {
+		return fmt.Sprintf("the opening `<%s>` tag is not closed; add `>` or `/>` before the code fence",
+			EmbeddingTag,
+		)
+	}
+	if !instructionClosed(instructionBody) {
+		return fmt.Sprintf("the `<%s>` instruction is not closed; add `</%s>` or use `/>`",
+			EmbeddingTag,
 			EmbeddingTag,
 		)
 	}
+
 	if parseErr != nil {
 		var syntaxErr *xml.SyntaxError
 		if errors.As(parseErr, &syntaxErr) {
@@ -160,3 +169,124 @@ func parseFailureReason(instructionBody []string, parseErr error) string {
 
 	return "invalid embedding instruction"
 }
+
+// openingTagClosed reports whether the first embed-code opening tag reaches `>`.
+func openingTagClosed(instructionBody []string) bool {
+	return scanInstructionBody(instructionBody, func(remaining string) (bool, bool) {
+		switch {
+		case strings.HasPrefix(remaining, ">"):
+			return true, true
+		case strings.HasPrefix(remaining, "</"+EmbeddingTag):
+			return false, true
+		case strings.HasPrefix(remaining, "<"+EmbeddingTag):
+			return false, true
+		default:
+			return false, false
+		}
+	})
+}
+
+// instructionClosed reports whether the instruction has a self-closing or closing tag.
+func instructionClosed(instructionBody []string) bool {
+	return scanInstructionBody(instructionBody, func(remaining string) (bool, bool) {
+		switch {
+		case strings.HasPrefix(remaining, "/>"):
+			return true, true
+		case strings.HasPrefix(remaining, "</"+EmbeddingTag+">"):
+			return true, true
+		default:
+			return false, false
+		}
+	})
+}
+
+// scanInstructionBody scans instruction text outside quoted attribute values.
+func scanInstructionBody(
+	instructionBody []string,
+	check func(remaining string) (bool, bool),
+) bool {
+	inTag := false
+	inQuote := false
+	var quote byte
+	for _, line := range instructionBody {
+		if inTag && !inQuote && markdownCodeFenceStart(line) {
+			return false
+		}
+		matched, done := scanInstructionLine(line, check, &inTag, &inQuote, &quote)
+		if done {
+			return matched
+		}
+	}
+
+	return false
+}
+
+// scanInstructionLine scans one instruction line outside quoted attribute values.
+func scanInstructionLine(
+	line string,
+	check func(remaining string) (bool, bool),
+	inTag *bool,
+	inQuote *bool,
+	quote *byte,
+) (bool, bool) {
+	for lineCursor := 0; lineCursor < len(line); lineCursor++ {
+		if !*inTag {
+			if !startInstructionScan(line, &lineCursor, inTag) {
+				break
+			}
+
+			continue
+		}
+		if *inQuote {
+			skipQuotedCharacter(line, &lineCursor, inQuote, quote)
+
+			continue
+		}
+		if line[lineCursor] == '"' || line[lineCursor] == '\'' {
+			*inQuote = true
+			*quote = line[lineCursor]
+
+			continue
+		}
+		matched, done := check(line[lineCursor:])
+		if done {
+			return matched, true
+		}
+	}
+
+	return false, false
+}
+
+// startInstructionScan moves lineCursor to the first embed-code tag.
+func startInstructionScan(line string, lineCursor *int, inTag *bool) bool {
+	tagStart := strings.Index(line[*lineCursor:], "<"+EmbeddingTag)
+	if tagStart < 0 {
+		return false
+	}
+	*lineCursor += tagStart + len("<"+EmbeddingTag) - 1
+	*inTag = true
+
+	return true
+}
+
+// skipQuotedCharacter updates quote state while scanning an attribute value.
+func skipQuotedCharacter(line string, lineCursor *int, inQuote *bool, quote *byte) {
+	if line[*lineCursor] == '\\' {
+		*lineCursor++
+
+		return
+	}
+	if line[*lineCursor] == *quote {
+		*inQuote = false
+	}
+}
+
+// markdownCodeFenceStart reports whether a line starts a Markdown code fence.
+func markdownCodeFenceStart(line string) bool {
+	trimmedLine := strings.TrimSpace(line)
+	if !strings.HasPrefix(trimmedLine, "```") {
+		return false
+	}
+
+	return len(codeFenceMarker(trimmedLine)) >= minCodeFenceMarkerLength
+}