Skip to content

Commit

Permalink
fix: Require newline in all cases for props on block sequence (fixes #…
Browse files Browse the repository at this point in the history
  • Loading branch information
eemeli committed Jun 29, 2024
1 parent 1b8fde6 commit aea700d
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 8 deletions.
26 changes: 23 additions & 3 deletions src/compose/compose-collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,40 @@ function resolveCollection(
return coll
}

interface Props {
anchor: SourceToken | null
tag: SourceToken | null
newlineAfterProp: SourceToken | null
}

export function composeCollection(
CN: ComposeNode,
ctx: ComposeContext,
token: BlockMap | BlockSequence | FlowCollection,
tagToken: SourceToken | null,
props: Props,
onError: ComposeErrorHandler
) {
const tagToken = props.tag
const tagName: string | null = !tagToken
? null
: ctx.directives.tagName(tagToken.source, msg =>
onError(tagToken, 'TAG_RESOLVE_FAILED', msg)
)

if (token.type === 'block-seq') {
const { anchor, newlineAfterProp: nl } = props
const lastProp =
anchor && tagToken
? anchor.offset > tagToken.offset
? anchor
: tagToken
: anchor ?? tagToken
if (lastProp && (!nl || nl.offset < lastProp.offset)) {
const message = 'Missing newline after block sequence props'
onError(lastProp, 'MISSING_CHAR', message)
}
}

const expType: 'map' | 'seq' =
token.type === 'block-map'
? 'map'
Expand All @@ -72,8 +93,7 @@ export function composeCollection(
!tagName ||
tagName === '!' ||
(tagName === YAMLMap.tagName && expType === 'map') ||
(tagName === YAMLSeq.tagName && expType === 'seq') ||
!expType
(tagName === YAMLSeq.tagName && expType === 'seq')
) {
return resolveCollection(CN, ctx, token, onError, tagName)
}
Expand Down
3 changes: 2 additions & 1 deletion src/compose/compose-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ interface Props {
comment: string
anchor: SourceToken | null
tag: SourceToken | null
newlineAfterProp: SourceToken | null
end: number
}

Expand Down Expand Up @@ -57,7 +58,7 @@ export function composeNode(
case 'block-map':
case 'block-seq':
case 'flow-collection':
node = composeCollection(CN, ctx, token, tag, onError)
node = composeCollection(CN, ctx, token, props, onError)
if (anchor) node.anchor = anchor.source.substring(1)
break
default: {
Expand Down
2 changes: 1 addition & 1 deletion src/compose/resolve-block-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export function resolveBlockMap(
}
continue
}
if (keyProps.hasNewlineAfterProp || containsNewline(key)) {
if (keyProps.newlineAfterProp || containsNewline(key)) {
onError(
key ?? start[start.length - 1],
'MULTILINE_IMPLICIT_KEY',
Expand Down
6 changes: 3 additions & 3 deletions src/compose/resolve-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export function resolveProps(
let comment = ''
let commentSep = ''
let hasNewline = false
let hasNewlineAfterProp = false
let reqSpace = false
let tab: SourceToken | null = null
let anchor: SourceToken | null = null
let tag: SourceToken | null = null
let newlineAfterProp: SourceToken | null = null
let comma: SourceToken | null = null
let found: SourceToken | null = null
let start: number | null = null
Expand Down Expand Up @@ -92,7 +92,7 @@ export function resolveProps(
} else commentSep += token.source
atNewline = true
hasNewline = true
if (anchor || tag) hasNewlineAfterProp = true
if (anchor || tag) newlineAfterProp = token
hasSpace = true
break
case 'anchor':
Expand Down Expand Up @@ -189,9 +189,9 @@ export function resolveProps(
spaceBefore,
comment,
hasNewline,
hasNewlineAfterProp,
anchor,
tag,
newlineAfterProp,
end,
start: start ?? end
}
Expand Down
25 changes: 25 additions & 0 deletions tests/doc/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,31 @@ describe('tags on invalid nodes', () => {
})
})

describe('properties on block sequences without newline after props', () => {
test('valid properties with newline', () => {
const doc = YAML.parseDocument('! &a\n- b')
expect(doc.errors).toMatchObject([])
})

test('properties on sequence without newline', () => {
const doc = YAML.parseDocument('!\n&a - b')
expect(doc.errors).toMatchObject([
{ code: 'MISSING_CHAR' },
{ code: 'UNEXPECTED_TOKEN' }
])
})

test('properties on empty sequence without newline', () => {
const doc = YAML.parseDocument('&a\n! -')
expect(doc.errors).toMatchObject([{ code: 'MISSING_CHAR' }])
})

test('properties on sequence with newline after item indicator', () => {
const doc = YAML.parseDocument('!\n&a -\n b')
expect(doc.errors).toMatchObject([{ code: 'MISSING_CHAR' }])
})
})

describe('invalid options', () => {
test('unknown schema', () => {
expect(() => new YAML.Document(undefined, { schema: 'foo' })).toThrow(
Expand Down

0 comments on commit aea700d

Please sign in to comment.