Add error messages to parse result, improve test coverage

Author: hippotastic

packages/annotation-comments/README.md

@@ -41,7 +41,7 @@ console.log('Some code');
 
 const codeLines = code.trim().split(/\r?\n/);
 
-const annotationComments = parseAnnotationComments({ codeLines });
+const { annotationComments, errorMessages } = parseAnnotationComments({ codeLines });
 
 cleanCode({ annotationComments, codeLines });
 ```
@@ -62,9 +62,14 @@ type ParseAnnotationCommentsOptions = {
 }
 ```
 
-Its return value is an array of `AnnotationComment` objects:
+Its return value is an object that contains both all parsed annotation comments and any error messages that occurred during parsing:
 
 ```ts
+export type ParseAnnotationCommentsResult = {
+  annotationComments: AnnotationComment[]
+  errorMessages: string[]
+}
+
 type AnnotationComment = {
   tag: AnnotationTag
   contents: string[]
@@ -95,6 +100,9 @@ export type SourceLocation = {
 
 ### cleanCode()
 
+> **Warning**
+> This function is not implemented yet.
+
 This function prepares annotated code lines for display or copying to the clipboard by making it look like regular (non-annotated) code again.
 
 It will collect all necessary edits and apply them to the code in reverse order (from the last edit location to the first) to avoid having to update the locations of all remaining annotations after each edit.

packages/annotation-comments/src/core/parse.ts

@@ -3,17 +3,23 @@ import { parseAnnotationTags } from '../parsers/annotation-tags'
 import { parseParentComment } from '../parsers/parent-comment'
 import { secondRangeIsInFirst } from '../internal/ranges'
 import { findAnnotationTargets } from './find-targets'
+import { coerceError } from '../internal/errors'
 
 export type ParseAnnotationCommentsOptions = {
 	codeLines: string[]
 }
 
-export function parseAnnotationComments(options: ParseAnnotationCommentsOptions): AnnotationComment[] {
+export type ParseAnnotationCommentsResult = {
+	annotationComments: AnnotationComment[]
+	errorMessages: string[]
+}
+
+export function parseAnnotationComments(options: ParseAnnotationCommentsOptions): ParseAnnotationCommentsResult {
 	const { codeLines } = options
 	const annotationComments: AnnotationComment[] = []
 
 	// Find annotation tags in the code
-	const annotationTags = parseAnnotationTags({ codeLines })
+	const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
 
 	const ignoreCountPerTagName = new Map<string, number>()
 	let tagsIgnoredUntilLineIndex = -1
@@ -36,21 +42,31 @@ export function parseAnnotationComments(options: ParseAnnotationCommentsOptions)
 			ignoreCountPerTagName.set(tag.name, ignoreCount - 1)
 			return
 		}
-		// Allow creating new ignores
+		// Allow creating new ignores through an `ignore-tags` annotation comment
 		if (tag.name === 'ignore-tags') {
-			// By definition, `ignore-tags` must be on its own line
-			if (comment.annotationRange.start.column || comment.annotationRange.end.column) return
-			const ignoreRange = tag.relativeTargetRange ?? 1
-			if (typeof tag.targetSearchQuery === 'string') {
-				const targetTagNames = tag.targetSearchQuery.split(',').map((name) => name.trim())
-				targetTagNames.forEach((name) => {
-					ignoreCountPerTagName.set(name, ignoreRange)
-				})
-			} else if (ignoreRange > 0) {
-				tagsIgnoredUntilLineIndex = tag.range.end.line + ignoreRange
+			try {
+				// Require ignore-tags to be on its own line
+				if (comment.annotationRange.start.column || comment.annotationRange.end.column) {
+					throw new Error('It must be on its own line.')
+				}
+				if (tag.relativeTargetRange !== undefined && !(tag.relativeTargetRange > 0)) {
+					throw new Error('If given, the target range must be a positive number.')
+				}
+				const ignoreRange = tag.relativeTargetRange ?? 1
+				if (typeof tag.targetSearchQuery === 'string') {
+					const targetTagNames = tag.targetSearchQuery.split(',').map((name) => name.trim())
+					targetTagNames.forEach((name) => {
+						ignoreCountPerTagName.set(name, ignoreRange)
+					})
+				} else if (ignoreRange > 0) {
+					tagsIgnoredUntilLineIndex = tag.range.end.line + ignoreRange
+				}
+				// We still want to add this valid ignore-tags comment to the result array
+				// to allow it to be removed by `cleanCode()` later, so we don't return here
+			} catch (error) {
+				const errorMessage = coerceError(error).message
+				errorMessages.push(`Invalid "ignore-tags" annotation in line ${comment.annotationRange.start.line + 1}: ${errorMessage}`)
 			}
-			// We still want to add this valid ignore-tags comment to the result array
-			// to allow it to be removed by `cleanCode()` later, so we don't return here
 		}
 
 		// If we arrive here, add the tag and comment to the list of annotation comments
@@ -61,5 +77,8 @@ export function parseAnnotationComments(options: ParseAnnotationCommentsOptions)
 	// Find the target ranges for all annotations
 	findAnnotationTargets({ codeLines, annotationComments })
 
-	return annotationComments
+	return {
+		annotationComments,
+		errorMessages,
+	}
 }

packages/annotation-comments/src/index.ts

@@ -1,6 +1,3 @@
 export type * from './core/types'
 export * from './core/parse'
 // export * from './core/clean'
-
-export * from './parsers/annotation-tags'
-export * from './parsers/parent-comment'

packages/annotation-comments/src/internal/errors.ts

@@ -6,7 +6,11 @@
  */
 export function coerceError(error: unknown): { message: string; code?: string | undefined } {
 	if (typeof error === 'object' && error !== null && 'message' in error) {
+		/* c8 ignore start */
 		return error as { message: string; code?: string | undefined }
+		/* c8 ignore end */
 	}
+	/* c8 ignore start */
 	return { message: error as string }
+	/* c8 ignore end */
 }

packages/annotation-comments/src/internal/regexps.ts

@@ -39,6 +39,7 @@ export function getGroupIndicesFromRegExpMatch(match: RegExpMatchArray) {
 	// Read the start and end ranges from the `indices` property,
 	// which is made available through the RegExp flag `d`
 	let groupIndices = match.indices as ([start: number, end: number] | null)[]
+	/* v8 ignore start */
 	if (groupIndices?.length) return groupIndices
 
 	// We could not access native group indices, so we need to use fallback logic
@@ -53,6 +54,7 @@ export function getGroupIndicesFromRegExpMatch(match: RegExpMatchArray) {
 	})
 
 	return groupIndices
+	/* v8 ignore end */
 }
 
 /**

packages/annotation-comments/src/parsers/annotation-tags.ts

@@ -1,11 +1,17 @@
 import { AnnotationTag } from '../core/types'
+import { coerceError } from '../internal/errors'
 import { getEscapeSequenceRegExp } from '../internal/escaping'
 import { createGlobalRegExp } from '../internal/regexps'
 
 export type ParseAnnotationTagsOptions = {
 	codeLines: string[]
 }
 
+export type ParseAnnotationTagsResult = {
+	annotationTags: AnnotationTag[]
+	errorMessages: string[]
+}
+
 const annotationTagRegex = new RegExp(
 	[
 		// Opening sequence
@@ -98,30 +104,40 @@ function parseTargetSearchQuery(rawTargetSearchQuery: string | undefined): strin
 	return unescapedQuery
 }
 
-export function parseAnnotationTags(options: ParseAnnotationTagsOptions): AnnotationTag[] {
+export function parseAnnotationTags(options: ParseAnnotationTagsOptions): ParseAnnotationTagsResult {
 	const { codeLines } = options
 	const annotationTags: AnnotationTag[] = []
+	const errorMessages: string[] = []
 
 	codeLines.forEach((line, lineIndex) => {
 		const matches = [...line.matchAll(annotationTagRegex)]
 		matches.forEach((match) => {
-			const [, name, rawTargetSearchQuery, relativeTargetRange] = match
-			const rawTag = match[0]
-			const startColIndex = match.index
-			const endColIndex = startColIndex + rawTag.length
-			const targetSearchQuery = parseTargetSearchQuery(rawTargetSearchQuery)
-			annotationTags.push({
-				name,
-				targetSearchQuery,
-				relativeTargetRange: relativeTargetRange !== undefined ? Number(relativeTargetRange) : undefined,
-				rawTag,
-				range: {
-					start: { line: lineIndex, column: startColIndex },
-					end: { line: lineIndex, column: endColIndex },
-				},
-			})
+			try {
+				const [, name, rawTargetSearchQuery, relativeTargetRange] = match
+				const rawTag = match[0]
+				const startColIndex = match.index
+				const endColIndex = startColIndex + rawTag.length
+
+				const targetSearchQuery = parseTargetSearchQuery(rawTargetSearchQuery)
+				annotationTags.push({
+					name,
+					targetSearchQuery,
+					relativeTargetRange: relativeTargetRange !== undefined ? Number(relativeTargetRange) : undefined,
+					rawTag,
+					range: {
+						start: { line: lineIndex, column: startColIndex },
+						end: { line: lineIndex, column: endColIndex },
+					},
+				})
+			} catch (error) {
+				const msg = coerceError(error).message
+				errorMessages.push(`Failed to parse annotation tag in line ${lineIndex + 1}: ${msg}`)
+			}
 		})
 	})
 
-	return annotationTags
+	return {
+		annotationTags,
+		errorMessages,
+	}
 }

packages/annotation-comments/src/parsers/comment-types/single-line.ts

@@ -120,7 +120,7 @@ export function parseSingleLineParentComment(options: ParseParentCommentOptions)
 					// and expand the comment range to cover the additional full line
 					const column = Math.min(possibleContentStart, line.length)
 					contents.push('')
-					contentRanges.push({ start: { line: lineIndex, column }, end: { line: lineIndex, column } })
+					contentRanges.push(createRange({ codeLines, start: { line: lineIndex, column }, end: { line: lineIndex } }))
 					commentRange.end = { line: lineIndex }
 				}
 			}

packages/annotation-comments/test/annotation-tags.test.ts

@@ -6,16 +6,76 @@ import { splitCodeLines } from './utils'
 
 describe('parseAnnotationTags()', () => {
 	test('Returns an empty array when no annotation tags are found', () => {
-		expect(
-			getTags(`
-// This is a simple comment.
-console.log('Some code');
+		const codeLines = [
+			// Example code without an annotation tag
+			'// This is a simple comment.',
+			'console.log("Some code");',
+			'',
+			'/*',
+			'\t[TODO] Not an annotation tag.',
+			'*/',
+		]
+		const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
 
-/*
-	[TODO] Not an annotation tag.
-*/
-			`)
-		).toEqual([])
+		expect(annotationTags).toEqual([])
+		expect(errorMessages).toEqual([])
+	})
+
+	describe('Returns error messages when invalid annotation tags are found', () => {
+		test('Single invalid regular expression', () => {
+			const codeLines = [
+				// The following regular expression should cause an error to be returned
+				'// [!note:/(this|group|is|unclosed/] Note the invalid regexp in the search query.',
+				'console.log("Some code");',
+			]
+			const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
+
+			expect(annotationTags).toEqual([])
+			expect(errorMessages).toEqual([
+				// Expect the line number and regexp contents to be called out
+				expect.stringMatching(/line 1.*\(this\|group/),
+			])
+		})
+		test('Multiple invalid regular expressions', () => {
+			const codeLines = [
+				// The following regular expression should cause an error to be returned
+				'// [!note:/(this|group|is|unclosed/] Note the invalid regexp in the search query.',
+				'console.log("Some code");',
+				'// [!note:/te[st/] This regexp is also invalid.',
+				'console.log("Some more code");',
+			]
+			const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
+
+			expect(annotationTags).toEqual([])
+			expect(errorMessages).toEqual([
+				// Expect both lines and regexp contents to be called out
+				expect.stringMatching(/line 1.*\(this\|group/),
+				expect.stringMatching(/line 3.*te\[st/),
+			])
+		})
+		test('Valid tags are still parsed even if invalid ones are also present', () => {
+			const codeLines = [
+				// The following regular expression should cause an error to be returned
+				'// [!note:/(this|group|is|unclosed/] Note the invalid regexp in the search query.',
+				'console.log("Some code"); // [!tag] This is a valid tag.',
+				'// [!note:/te[st/] This regexp is also invalid.',
+				'console.log("Some more code");',
+			]
+			const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
+
+			expect(annotationTags).toEqual([
+				expect.objectContaining({
+					name: 'tag',
+					targetSearchQuery: undefined,
+					relativeTargetRange: undefined,
+				}),
+			])
+			expect(errorMessages).toEqual([
+				// Expect both lines and regexp contents to be called out
+				expect.stringMatching(/line 1.*\(this\|group/),
+				expect.stringMatching(/line 3.*te\[st/),
+			])
+		})
 	})
 
 	describe('Parses tag names', () => {
@@ -250,8 +310,8 @@ console.log('Some code'); // ${test.rawTag} Tag at the end of a line
 ${test.rawTag} Tag in a multi-line comment
 */
 		`)
-		// Build expected result by searching the raw tag in the code lines
-		const expected = codeLines.reduce((acc, line, lineIndex) => {
+		// Build array of expected tags by searching the raw tag in the code lines
+		const expectedAnnotationTags = codeLines.reduce((acc, line, lineIndex) => {
 			let startIndex = line.indexOf(test.rawTag)
 			while (startIndex !== -1) {
 				acc.push({
@@ -269,11 +329,8 @@ ${test.rawTag} Tag in a multi-line comment
 			return acc
 		}, [] as AnnotationTag[])
 		// Perform test
-		expect(parseAnnotationTags({ codeLines })).toEqual(expected)
-	}
-
-	function getTags(code: string) {
-		const codeLines = splitCodeLines(code)
-		return parseAnnotationTags({ codeLines })
+		const { annotationTags, errorMessages } = parseAnnotationTags({ codeLines })
+		expect(annotationTags).toEqual(expectedAnnotationTags)
+		expect(errorMessages).toEqual([])
 	}
 })