-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enforce consistent punctuation in algorithms #1069
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
import { readFile, readdir } from "node:fs/promises"; | ||
|
||
const SPEC_DIR = new URL("../spec", import.meta.url).pathname; | ||
|
||
process.exitCode = 0; | ||
const filenames = await readdir(SPEC_DIR); | ||
for (const filename of filenames) { | ||
if (!filename.endsWith(".md")) { | ||
continue; | ||
} | ||
const markdown = await readFile(`${SPEC_DIR}/${filename}`, "utf8"); | ||
|
||
/** | ||
* Not strictly 'lines' since we try and group indented things together as if | ||
* they were one line. Close enough though. | ||
*/ | ||
const lines = markdown.split(/\n(?=[\S\n]|\s*(?:-|[0-9]+\.) )/); | ||
|
||
for (let i = 0, l = lines.length; i < l; i++) { | ||
const line = lines[i]; | ||
|
||
// Check algorithm is consistently formatted | ||
{ | ||
// Is it an algorithm definition? | ||
const matches = line.match(/^([a-z0-9A-Z]+)(\s*)\(([^)]*)\)(\s*):(\s*)$/); | ||
if (matches) { | ||
const [, algorithmName, ns1, _args, ns2, ns3] = matches; | ||
if (ns1 || ns2 || ns3) { | ||
console.log( | ||
`Bad whitespace in definition of ${algorithmName} in '${filename}':` | ||
); | ||
console.log(line); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
if (lines[i + 1] !== "") { | ||
console.log( | ||
`No empty space after algorithm ${algorithmName} header in '${filename}'` | ||
); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
for (let j = i + 2; j < l; j++) { | ||
const step = lines[j]; | ||
if (!step.match(/^\s*(-|[0-9]+\.) /)) { | ||
if (step !== "") { | ||
console.log( | ||
`Bad algorithm ${algorithmName} step in '${filename}':` | ||
); | ||
console.log(step); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
break; | ||
} | ||
if (!step.match(/[.:]$/)) { | ||
console.log( | ||
`Bad formatting for '${algorithmName}' step (does not end in '.' or ':') in '${filename}':` | ||
); | ||
console.log(step); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
if (step.match(/^\s*(-|[0-9]\.)\s+[a-z]/)) { | ||
console.log( | ||
`Bad formatting of '${algorithmName}' step (should start with a capital) in '${filename}':` | ||
); | ||
console.log(step); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
const trimmedInnerLine = step.replace(/\s+/g, " "); | ||
if ( | ||
trimmedInnerLine.match( | ||
/(?:[rR]eturn|is (?:not )?)(true|false|null)\b/ | ||
) && | ||
!trimmedInnerLine.match(/null or empty/) | ||
) { | ||
console.log( | ||
`Potential bad formatting of '${algorithmName}' step (true/false/null should be wrapped in curly braces, e.g. '{true}') in '${filename}':` | ||
); | ||
console.log(step); | ||
console.log(); | ||
process.exitCode = 1; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Check `- ...:` step is followed by an indent | ||
{ | ||
const matches = line.match(/^(\s*)- .*:\s*$/); | ||
if (matches) { | ||
const indent = matches[1]; | ||
const nextLine = lines[i + 1]; | ||
if (!nextLine.startsWith(`${indent} `)) { | ||
console.log( | ||
`Lacking indent in '${filename}' following ':' character:` | ||
); | ||
console.dir(line); | ||
console.dir(nextLine); | ||
console.log(); | ||
// TODO: process.exitCode = 1; | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
if (process.exitCode === 0) { | ||
console.log(`Everything looks okay!`); | ||
} else { | ||
console.log(`Please resolve the errors detailed above.`); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1032,7 +1032,7 @@ BlockStringValue(rawValue): | |
- Let {lines} be the result of splitting {rawValue} by {LineTerminator}. | ||
- Let {commonIndent} be {null}. | ||
- For each {line} in {lines}: | ||
- If {line} is the first item in {lines}, continue to the next line. | ||
- If {line} is the first item in {lines}, continue to the next {line}. | ||
- Let {length} be the number of characters in {line}. | ||
- Let {indent} be the number of leading consecutive {WhiteSpace} characters in | ||
{line}. | ||
|
@@ -1117,7 +1117,7 @@ ListValue : [ ] | |
ListValue : [ Value+ ] | ||
|
||
- Let {inputList} be a new empty list value. | ||
- For each {Value+} | ||
- For each {Value+}: | ||
- Let {value} be the result of evaluating {Value}. | ||
- Append {value} to {inputList}. | ||
- Return {inputList}. | ||
|
@@ -1164,7 +1164,7 @@ ObjectValue : { } | |
ObjectValue : { ObjectField+ } | ||
|
||
- Let {inputObject} be a new input object value with no fields. | ||
- For each {field} in {ObjectField+} | ||
- For each {field} in {ObjectField+}: | ||
- Let {name} be {Name} in {field}. | ||
- Let {value} be the result of evaluating {Value} in {field}. | ||
- Add a field to {inputObject} of name {name} containing value {value}. | ||
|
@@ -1247,22 +1247,22 @@ input type. | |
|
||
Type : Name | ||
|
||
- Let {name} be the string value of {Name} | ||
- Let {name} be the string value of {Name}. | ||
- Let {type} be the type defined in the Schema named {name} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why no full stop on that line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is a great question. I wonder how this escaped the editing... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah; it's because this isn't detected as an algorithm, algorithms start |
||
- {type} must not be {null} | ||
- Return {type} | ||
- {type} must not be {null}. | ||
- Return {type}. | ||
|
||
Type : [ Type ] | ||
|
||
- Let {itemType} be the result of evaluating {Type} | ||
- Let {itemType} be the result of evaluating {Type}. | ||
- Let {type} be a List type where {itemType} is the contained type. | ||
- Return {type} | ||
- Return {type}. | ||
|
||
Type : Type ! | ||
|
||
- Let {nullableType} be the result of evaluating {Type} | ||
- Let {nullableType} be the result of evaluating {Type}. | ||
- Let {type} be a Non-Null type where {nullableType} is the contained type. | ||
- Return {type} | ||
- Return {type}. | ||
|
||
## Directives | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -347,22 +347,22 @@ can only be used as input types. Object, Interface, and Union types can only be | |
used as output types. Lists and Non-Null types may be used as input types or | ||
output types depending on how the wrapped type may be used. | ||
|
||
IsInputType(type) : | ||
IsInputType(type): | ||
|
||
- If {type} is a List type or Non-Null type: | ||
- Let {unwrappedType} be the unwrapped type of {type}. | ||
- Return IsInputType({unwrappedType}) | ||
- Return IsInputType({unwrappedType}). | ||
- If {type} is a Scalar, Enum, or Input Object type: | ||
- Return {true} | ||
- Return {true}. | ||
- Return {false}. | ||
|
||
IsOutputType(type) : | ||
IsOutputType(type): | ||
|
||
- If {type} is a List type or Non-Null type: | ||
- Let {unwrappedType} be the unwrapped type of {type}. | ||
- Return IsOutputType({unwrappedType}) | ||
- Return IsOutputType({unwrappedType}). | ||
- If {type} is a Scalar, Object, Interface, Union, or Enum type: | ||
- Return {true} | ||
- Return {true}. | ||
- Return {false}. | ||
|
||
### Type Extensions | ||
|
@@ -919,7 +919,7 @@ of rules must be adhered to by every Object type in a GraphQL schema. | |
3. The argument must accept a type where {IsInputType(argumentType)} | ||
returns {true}. | ||
4. If argument type is Non-Null and a default value is not defined: | ||
- The `@deprecated` directive must not be applied to this argument. | ||
1. The `@deprecated` directive must not be applied to this argument. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would the unordered list be preferred in this case? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIRC, it would be the only unordered item in any of our equivalent algorithms, so I changed it for consistency. |
||
3. An object type may declare that it implements one or more unique interfaces. | ||
4. An object type must be a super-set of all interfaces it implements: | ||
1. Let this object type be {objectType}. | ||
|
@@ -1699,7 +1699,7 @@ input ExampleInputObject { | |
3. The input field must accept a type where {IsInputType(inputFieldType)} | ||
returns {true}. | ||
4. If input field type is Non-Null and a default value is not defined: | ||
- The `@deprecated` directive must not be applied to this input field. | ||
1. The `@deprecated` directive must not be applied to this input field. | ||
3. If an Input Object references itself either directly or through referenced | ||
Input Objects, at least one of the fields in the chain of references must be | ||
either a nullable or a List type. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This TODO is due to bad formatting in
MapSourceToResponseEvent
, but that's something that's a little more controversial so I raised a separate PR for it and removed the TODO here:2d1c477