Skip to content

Commit

Permalink
Stop forcing shorthand args to be 'id' (#100)
Browse files Browse the repository at this point in the history
- Shorthand arg (i.e. the name after a tag instead of an attribute list) is not necessarily an "id"
- Refactor: 'validator' actually a part of parser family, not processor.
- The shorthand change revealed validator is using parts of the processor when it should not.
- Tags now have an option for what to call the shorthand.
  • Loading branch information
cbush authored May 20, 2022
1 parent 95dba02 commit 3c81783
Show file tree
Hide file tree
Showing 16 changed files with 125 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/bluehawk/bluehawk.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { idIsUnique } from "./processor/validator";
import { idIsUnique } from "./parser/validator";
import { Bluehawk } from "./bluehawk";
import { Document } from "./Document";
import {
Expand Down
14 changes: 4 additions & 10 deletions src/bluehawk/bluehawk.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ConsoleActionReporter } from "./actions/ConsoleActionReporter";
import { ActionReporter } from "./actions/ActionReporter";
import { validateTags } from "./processor/validator";
import { TAG_PATTERN } from "./parser/lexer/tokens";
import { Document } from "./Document";
import {
Expand Down Expand Up @@ -186,15 +185,10 @@ This is probably a bug in Bluehawk. Please send this stack trace (and the conten
});
parser = this._parserStore.getDefaultParser();
}
const result = parser.parse(source);
const validateErrors = validateTags(
result.tagNodes,
this._processor.processors
);
return {
...result,
errors: [...result.errors, ...validateErrors],
};
return parser.parse({
source,
tagProcessors: this._processor.processors,
});
};

/**
Expand Down
1 change: 1 addition & 0 deletions src/bluehawk/getBluehawk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const getBluehawk = async (): Promise<Bluehawk> => {
const StateUncommentTag = makeBlockTag<IdRequiredAttributes>({
name: "state-uncomment",
description: "combines 'uncomment' and 'state'",
shorthandArgsAttributeName: "id",
attributesSchema: IdRequiredAttributesSchema,
process(request) {
UncommentTag.process(request);
Expand Down
8 changes: 4 additions & 4 deletions src/bluehawk/parser/TagNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ interface TagNode {
// The comment context the tag was found in.
inContext: TagNodeContext;

// Returns the id found in the attributes list or directly after the block
// tag.
id?: string[];

// Block tags have an inner range that includes the lines between the
// attribute list and the end tag token.
contentRange?: Range;
Expand All @@ -41,6 +37,9 @@ interface TagNode {

// Attributes come from JSON and their schema depends on the tag.
attributes?: TagNodeAttributes;

// Shorthand args are passed after a tag name.
shorthandArgs?: string[];
}

// A line tag applies to a specific line and does not have -start or -end
Expand Down Expand Up @@ -95,6 +94,7 @@ export class TagNodeImpl implements TagNode {
}
contentRange?: Range;
children?: TagNodeImpl[];
shorthandArgs?: string[];
attributes?: TagNodeAttributes;
newlines: IToken[] = [];
lineComments: IToken[] = [];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { JSONSchemaType } from "ajv";
import { TagNodeImpl, TagNodeAttributes, AnyTagNode } from "../parser/TagNode";
import { TagNodeImpl, TagNodeAttributes, AnyTagNode } from "./TagNode";
import { Range } from "../Range";
import { makeAttributesConformToJsonSchemaRule } from "./makeAttributesConformToJsonSchemaRule";
import { ValidateCstResult } from "./validator";
Expand Down
17 changes: 14 additions & 3 deletions src/bluehawk/parser/makeParser.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { TagProcessors } from "./../processor/Processor";
import { Document } from "../Document";
import { LanguageSpecification } from "./LanguageSpecification";
import {
Expand All @@ -9,11 +10,12 @@ import { makeStringLiteralToken } from "./lexer/makeStringLiteralToken";
import { ParseResult } from "./ParseResult";
import { RootParser } from "./RootParser";
import { makeCstVisitor } from "./visitor";
import { validateTags } from "./validator";

// Complete parser (lexicon, syntax, and semantics).
export interface IParser {
languageSpecification: LanguageSpecification;
parse(source: Document): ParseResult;
parse(args: { source: Document; tagProcessors?: TagProcessors }): ParseResult;
}

// Token patterns are required to be sticky (y flag)
Expand Down Expand Up @@ -58,7 +60,7 @@ export function makeParser(
const syntaxProcessor = new RootParser(languageTokens);
const semanticsProcessor = makeCstVisitor(syntaxProcessor);
return {
parse(source: Document) {
parse({ source, tagProcessors }) {
// ⚠️ Caller is responsible for making sure this parser is appropriate for
// the source language
const parseResult = syntaxProcessor.parse(source.text.original);
Expand All @@ -71,8 +73,17 @@ export function makeParser(
};
}
const visitorResult = semanticsProcessor.visit(parseResult.cst, source);

const validateErrors =
tagProcessors !== undefined
? validateTags(visitorResult.tagNodes, tagProcessors)
: [];
return {
errors: [...parseResult.errors, ...visitorResult.errors],
errors: [
...parseResult.errors,
...visitorResult.errors,
...validateErrors,
],
tagNodes: visitorResult.tagNodes,
source,
languageSpecification,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { strict as assert } from "assert";
import { RootParser } from "../parser/RootParser";
import { makeCstVisitor } from "../parser/visitor/makeCstVisitor";
import { RootParser } from "./RootParser";
import { makeCstVisitor } from "./visitor/makeCstVisitor";
import { validateTags, idIsUnique } from "./validator";
import { makeBlockCommentTokens } from "../parser/lexer/makeBlockCommentTokens";
import { makeLineCommentToken } from "../parser/lexer/makeLineCommentToken";
import { makeBlockCommentTokens } from "./lexer/makeBlockCommentTokens";
import { makeLineCommentToken } from "./lexer/makeLineCommentToken";
import { Document } from "../Document";
import { TagProcessors } from "./Processor";
import { TagProcessors } from "../processor/Processor";
import {
IdRequiredAttributes,
IdRequiredAttributesSchema,
Expand All @@ -21,6 +21,7 @@ describe("validator", () => {
"code-block": makeBlockTag<IdRequiredAttributes>({
name: "code-block",
attributesSchema: IdRequiredAttributesSchema,
shorthandArgsAttributeName: "id",
rules: [idIsUnique],
process(request) {
// do nothing
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { TagProcessors } from "./../processor/Processor";
import { AnyTagNode } from "../parser";
import { BluehawkError } from "../BluehawkError";
import { flatten } from "../parser";
import { TagProcessors } from "./Processor";
import { makeAttributesConformToJsonSchemaRule } from "./makeAttributesConformToJsonSchemaRule";
import { mapShorthandArgsToAttributes } from "../tags/Tag";

export interface ValidateCstResult {
errors: BluehawkError[];
Expand Down Expand Up @@ -31,6 +32,12 @@ export function validateTags(
return;
}

// Apply the tag's shorthand rename rule to the tag node's attribute list
mapShorthandArgsToAttributes({
tagNode,
shorthandArgsAttributeName: tag.shorthandArgsAttributeName,
});

if (tagNode.type === "block" && !tag.supportsBlockMode) {
validateResult.errors.push({
component: "validator",
Expand Down Expand Up @@ -69,22 +76,25 @@ export const idIsUnique: Rule = (
tagNode: AnyTagNode,
result: ValidateCstResult
) => {
if (tagNode.id !== undefined) {
if (tagNode.attributes?.id !== undefined) {
// if the tag id already exists in the set of tag ids, create a duplicate error
const union = [
...new Set([...Array.from(result.tagsById.keys()), ...tagNode.id]),
...new Set([
...Array.from(result.tagsById.keys()),
...tagNode.attributes.id,
]),
];
// if the union of the seen ids and the current node's ids has fewer elements than the length of those in total... duplicate
if (union.length != result.tagsById.size + tagNode.id.length) {
// (result.tagsById.has(tagNode.id)) {
if (union.length != result.tagsById.size + tagNode.attributes.id.length) {
// (result.tagsById.has(tagNode.attributes.id)) {
result.errors.push({
component: "validator",
location: tagNode.range.start,
message: `duplicate ID '${tagNode.id}' found`,
message: `duplicate ID '${tagNode.attributes.id}' found`,
});
} else {
// otherwise, add the tag id to the set
tagNode.id.forEach((id) => {
tagNode.attributes.id.forEach((id: string) => {
result.tagsById.set(id, tagNode);
});
}
Expand Down
11 changes: 5 additions & 6 deletions src/bluehawk/parser/visitor/makeCstVisitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,11 @@ export function makeCstVisitor(
const Identifier = context.Identifier;
const attributeList = context.attributeList;
if (Identifier != undefined && Identifier.length != 0) {
assert(!attributeList); // parser issue
parent.attributes = {
id: Identifier.map((identifier) => identifier.image),
};
} else if (attributeList !== undefined) {
assert(!Identifier);
parent.shorthandArgs = Identifier.map(
(identifier) => identifier.image
);
}
if (attributeList !== undefined) {
assert(attributeList.length === 1); // should be impossible to have more than 1 list
this.visit(attributeList, visitorContext);
}
Expand Down
6 changes: 3 additions & 3 deletions src/bluehawk/parser/visitor/visitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ annotated text
expect(result.tagNodes[0].tagName).toBe("A");
expect((result.tagNodes[0].children ?? []).length).toBe(0);
expect(
result.tagNodes[0].id !== undefined &&
result.tagNodes[0].id.length == 1 &&
result.tagNodes[0].id[0]
result.tagNodes[0].shorthandArgs !== undefined &&
result.tagNodes[0].shorthandArgs.length == 1 &&
result.tagNodes[0].shorthandArgs[0]
).toBe("label");
});

Expand Down
5 changes: 3 additions & 2 deletions src/bluehawk/processor/Processor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ This is probably not a bug in the Bluehawk library itself. Please check with the
expect(tagNode.children).toBeUndefined();
expect(tagNode.contentRange).toBeUndefined();
expect(tagNode.tagName).toBe("line-tag");
expect(tagNode.id).toBeUndefined();
expect(tagNode.shorthandArgs).toBeUndefined();
expect(tagNode.type).toBe("line");
state.calledLineTagProcess = true;
},
Expand All @@ -148,11 +148,12 @@ This is probably not a bug in the Bluehawk library itself. Please check with the
const BlockTag = makeBlockTag<IdRequiredAttributes>({
name: "block-tag",
attributesSchema: IdRequiredAttributesSchema,
shorthandArgsAttributeName: "id",
process({ tagNode }) {
expect(tagNode.attributes).toBeDefined();
expect(tagNode.children).toBeDefined();
expect(tagNode.contentRange).toBeDefined();
expect(tagNode.id).toBe("test");
expect(tagNode.attributes.id).toStrictEqual(["test"]);
expect(tagNode.type).toBe("block");
state.calledBlockTagProcess = true;
},
Expand Down
11 changes: 7 additions & 4 deletions src/bluehawk/tags/SnippetTag.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import MagicString from "magic-string";
import { BlockTagNode } from "../parser/TagNode";
import { ProcessRequest } from "../processor/Processor";
import { idIsUnique } from "../processor/validator";
import { idIsUnique } from "../parser/validator";
import { strict as assert } from "assert";
import {
makeBlockTag,
Expand Down Expand Up @@ -66,6 +66,7 @@ export const SnippetTag = makeBlockTag<IdRequiredAttributes>({
name: "snippet",
description: "identifies snippets for extraction into standalone files",
attributesSchema: IdRequiredAttributesSchema,
shorthandArgsAttributeName: "id",
rules: [idIsUnique],
process: (request: ProcessRequest<BlockTagNode>): void => {
const { tagNode, document, fork } = request;
Expand All @@ -86,16 +87,18 @@ export const SnippetTag = makeBlockTag<IdRequiredAttributes>({
dedentRange(clonedSnippet, tagNode);

// ID is required and enforced by the validator, so it should exist
assert(tagNode.id !== undefined && tagNode.id.length > 0);
assert(
tagNode.attributes.id !== undefined && tagNode.attributes.id.length > 0
);

// Fork subset code block to another file
fork({
document,
tagNodes: tagNode.children ?? [],
newPath: document.pathWithInfix(`codeblock.${tagNode.id[0]}`),
newPath: document.pathWithInfix(`codeblock.${tagNode.attributes.id[0]}`),
newText: clonedSnippet,
newAttributes: {
snippet: tagNode.id[0],
snippet: tagNode.attributes.id[0],
},
});
},
Expand Down
1 change: 1 addition & 0 deletions src/bluehawk/tags/StateTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export const StateTag = makeBlockTag<IdsRequiredAttributes>({
description:
"given a state name(s) as tag ids, identifies blocks that should only appear in the given state's version of the file",
attributesSchema: IdsRequiredAttributesSchema,
shorthandArgsAttributeName: "id",
process(request) {
const { tagNode, fork, document, tagNodes } = request;

Expand Down
64 changes: 56 additions & 8 deletions src/bluehawk/tags/Tag.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,48 @@
import { Rule } from "../processor/validator";
import { Rule } from "../parser/validator";
import { ProcessRequest } from "../processor/Processor";
import { AnySchema, JSONSchemaType } from "ajv";
import { AnyTagNode, BlockTagNode, LineTagNode } from "../parser";

interface Tag {
// The tag name. For block tags this should not include -start or
// -end.
/**
The tag name. For block tags this should not include -start or
-end.
*/
name: string;

// A helpful description of what the tag is supposed to do
/**
A helpful description of what the tag is supposed to do
*/
description?: string;

// JSON schema of the attributes list
/**
JSON schema of the attributes list
*/
attributesSchema?: AnySchema;

// Validator rules to determine if the tag meets requirements before
// processing is possible
/**
The attribute names that shorthand arguments map to. The position in the
array corresponds to the position of the argument.
Example: given shorthandArgsAttributeName = "id", the following
shorthand:
```
:some-tag-start: somename somename2
```
would map to the following attribute list:
```
:some-tag-start: { "id": ["somename", "somename2"] }
```
*/
shorthandArgsAttributeName?: string;

/**
Validator rules to determine if the tag meets requirements before
processing is possible
*/
rules?: Rule[];
}

Expand Down Expand Up @@ -78,7 +105,11 @@ export function makeBlockOrLineTag<AttributesType>(
process: (request: ProcessRequest<AnyTagNode>) => NotPromise;
}
): AnyTag {
return { ...tag, supportsLineMode: true, supportsBlockMode: true };
return {
...tag,
supportsLineMode: true,
supportsBlockMode: true,
};
}

// Helper for tags that require one and only one id
Expand Down Expand Up @@ -113,3 +144,20 @@ export const NoAttributesSchema: JSONSchemaType<NoAttributes> = {
type: "null",
nullable: true,
};

export function mapShorthandArgsToAttributes({
tagNode,
shorthandArgsAttributeName,
}: {
tagNode: AnyTagNode;
shorthandArgsAttributeName?: string;
}) {
if (
tagNode.shorthandArgs === undefined ||
shorthandArgsAttributeName === undefined
) {
return;
}
tagNode.attributes = tagNode.attributes ?? {};
tagNode.attributes[shorthandArgsAttributeName] = tagNode.shorthandArgs;
}
Loading

0 comments on commit 3c81783

Please sign in to comment.