Skip to content
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

ESLint: enable some of the rules previously blocked by TS conversion #3277

Merged
merged 1 commit into from Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 6 additions & 5 deletions .eslintrc.yml
Expand Up @@ -473,7 +473,8 @@ overrides:
'@typescript-eslint/ban-tslint-comment': error
'@typescript-eslint/ban-types': off # TODO temporarily disabled
'@typescript-eslint/class-literal-property-style': off # TODO enable after TS conversion
'@typescript-eslint/consistent-indexed-object-style': off # TODO enable after TS conversion
'@typescript-eslint/consistent-indexed-object-style':
[error, index-signature]
'@typescript-eslint/consistent-type-assertions': off # TODO temporarily disable
'@typescript-eslint/consistent-type-definitions': error
'@typescript-eslint/consistent-type-imports': error
Expand Down Expand Up @@ -507,7 +508,7 @@ overrides:
'@typescript-eslint/no-require-imports': error
'@typescript-eslint/no-this-alias': error
'@typescript-eslint/no-type-alias': off # TODO consider
'@typescript-eslint/no-unnecessary-boolean-literal-compare': off # FIXME requires on strictNullChecks
'@typescript-eslint/no-unnecessary-boolean-literal-compare': error
'@typescript-eslint/no-unnecessary-condition': off # TODO temporary disable
'@typescript-eslint/no-unnecessary-qualifier': error
'@typescript-eslint/no-unnecessary-type-arguments': error
Expand All @@ -522,9 +523,9 @@ overrides:
'@typescript-eslint/non-nullable-type-assertion-style': off #TODO temporarily disabled
'@typescript-eslint/prefer-as-const': error
'@typescript-eslint/prefer-enum-initializers': error
'@typescript-eslint/prefer-for-of': off # TODO switch to error after TS migration
'@typescript-eslint/prefer-for-of': error
'@typescript-eslint/prefer-function-type': error
'@typescript-eslint/prefer-includes': off # TODO switch to error after IE11 drop
'@typescript-eslint/prefer-includes': error
'@typescript-eslint/prefer-literal-enum-member': error
'@typescript-eslint/prefer-namespace-keyword': error
'@typescript-eslint/prefer-nullish-coalescing': error
Expand All @@ -534,7 +535,7 @@ overrides:
'@typescript-eslint/prefer-reduce-type-parameter': error
'@typescript-eslint/prefer-regexp-exec': off
'@typescript-eslint/prefer-return-this-type': error
'@typescript-eslint/prefer-string-starts-ends-with': off # TODO switch to error after IE11 drop
'@typescript-eslint/prefer-string-starts-ends-with': error
'@typescript-eslint/prefer-ts-expect-error': error
'@typescript-eslint/promise-function-async': off
'@typescript-eslint/require-array-sort-compare': error
Expand Down
10 changes: 5 additions & 5 deletions src/language/blockString.ts
Expand Up @@ -35,8 +35,8 @@ export function dedentBlockStringValue(rawString: string): string {
}

function isBlank(str: string): boolean {
for (let i = 0; i < str.length; ++i) {
if (str[i] !== ' ' && str[i] !== '\t') {
for (const char of str) {
if (char !== ' ' && char !== '\t') {
return false;
}
}
Expand Down Expand Up @@ -96,9 +96,9 @@ export function printBlockString(
preferMultipleLines: boolean = false,
): string {
const isSingleLine = !value.includes('\n');
const hasLeadingSpace = value[0] === ' ' || value[0] === '\t';
const hasTrailingQuote = value[value.length - 1] === '"';
const hasTrailingSlash = value[value.length - 1] === '\\';
const hasLeadingSpace = value.startsWith(' ') || value.startsWith('\t');
const hasTrailingQuote = value.endsWith('"');
const hasTrailingSlash = value.endsWith('\\');
const printAsMultipleLines =
!isSingleLine ||
hasTrailingQuote ||
Expand Down
31 changes: 18 additions & 13 deletions src/language/visitor.ts
Expand Up @@ -211,20 +211,25 @@ export function visit(
node = parent;
parent = ancestors.pop();
if (isEdited) {
node = inArray
? node.slice()
: Object.defineProperties({}, Object.getOwnPropertyDescriptors(node));
let editOffset = 0;
for (let ii = 0; ii < edits.length; ii++) {
let editKey: any = edits[ii][0];
const editValue = edits[ii][1];
if (inArray) {
editKey -= editOffset;
if (inArray) {
node = node.slice();

let editOffset = 0;
for (const [editKey, editValue] of edits) {
const arrayKey = editKey - editOffset;
if (editValue === null) {
node.splice(arrayKey, 1);
editOffset++;
} else {
node[arrayKey] = editValue;
}
}
if (inArray && editValue === null) {
node.splice(editKey, 1);
editOffset++;
} else {
} else {
node = Object.defineProperties(
{},
Object.getOwnPropertyDescriptors(node),
);
for (const [editKey, editValue] of edits) {
node[editKey] = editValue;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/utilities/assertValidName.ts
Expand Up @@ -20,7 +20,7 @@ export function assertValidName(name: string): string {
*/
export function isValidNameError(name: string): GraphQLError | undefined {
devAssert(typeof name === 'string', 'Expected name to be a string.');
if (name.length > 1 && name[0] === '_' && name[1] === '_') {
if (name.startsWith('__')) {
return new GraphQLError(
`Name "${name}" must not begin with "__", which is reserved by GraphQL introspection.`,
);
Expand Down
5 changes: 2 additions & 3 deletions src/utilities/stripIgnoredCharacters.ts
Expand Up @@ -112,9 +112,8 @@ function dedentBlockString(blockStr: string): string {
body = '\n' + body;
}

const lastChar = body[body.length - 1];
const hasTrailingQuote = lastChar === '"' && body.slice(-4) !== '\\"""';
if (hasTrailingQuote || lastChar === '\\') {
const hasTrailingQuote = body.endsWith('"') && !body.endsWith('\\"""');
if (hasTrailingQuote || body.endsWith('\\')) {
body += '\n';
}

Expand Down
4 changes: 2 additions & 2 deletions src/utilities/typedQueryDocumentNode.ts
Expand Up @@ -3,8 +3,8 @@ import type { DocumentNode, ExecutableDefinitionNode } from '../language/ast';
* Wrapper type that contains DocumentNode and types that can be deduced from it.
*/
export interface TypedQueryDocumentNode<
TResponseData = Record<string, any>,
TRequestVariables = Record<string, any>,
TResponseData = { [key: string]: any },
TRequestVariables = { [key: string]: any },
> extends DocumentNode {
readonly definitions: ReadonlyArray<ExecutableDefinitionNode>;
// FIXME: remove once TS implements proper way to enforce nominal typing
Expand Down
105 changes: 52 additions & 53 deletions src/validation/rules/OverlappingFieldsCanBeMergedRule.ts
Expand Up @@ -238,11 +238,12 @@ function collectConflictsBetweenFieldsAndFragment(
return;
}

const [fieldMap2, fragmentNames2] = getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment,
);
const [fieldMap2, referencedFragmentNames] =
getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment,
);

// Do not compare a fragment's fieldMap to itself.
if (fieldMap === fieldMap2) {
Expand All @@ -263,15 +264,15 @@ function collectConflictsBetweenFieldsAndFragment(

// (E) Then collect any conflicts between the provided collection of fields
// and any fragment names found in the given fragment.
for (let i = 0; i < fragmentNames2.length; i++) {
for (const referencedFragmentName of referencedFragmentNames) {
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap,
fragmentNames2[i],
referencedFragmentName,
);
}
}
Expand Down Expand Up @@ -310,16 +311,18 @@ function collectConflictsBetweenFragments(
return;
}

const [fieldMap1, fragmentNames1] = getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment1,
);
const [fieldMap2, fragmentNames2] = getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment2,
);
const [fieldMap1, referencedFragmentNames1] =
getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment1,
);
const [fieldMap2, referencedFragmentNames2] =
getReferencedFieldsAndFragmentNames(
context,
cachedFieldsAndFragmentNames,
fragment2,
);

// (F) First, collect all conflicts between these two collections of fields
// (not including any nested fragments).
Expand All @@ -335,28 +338,28 @@ function collectConflictsBetweenFragments(

// (G) Then collect conflicts between the first fragment and any nested
// fragments spread in the second fragment.
for (let j = 0; j < fragmentNames2.length; j++) {
for (const referencedFragmentName2 of referencedFragmentNames2) {
collectConflictsBetweenFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fragmentName1,
fragmentNames2[j],
referencedFragmentName2,
);
}

// (G) Then collect conflicts between the second fragment and any nested
// fragments spread in the first fragment.
for (let i = 0; i < fragmentNames1.length; i++) {
for (const referencedFragmentName1 of referencedFragmentNames1) {
collectConflictsBetweenFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fragmentNames1[i],
referencedFragmentName1,
fragmentName2,
);
}
Expand Down Expand Up @@ -403,49 +406,45 @@ function findConflictsBetweenSubSelectionSets(

// (I) Then collect conflicts between the first collection of fields and
// those referenced by each fragment name associated with the second.
if (fragmentNames2.length !== 0) {
for (let j = 0; j < fragmentNames2.length; j++) {
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap1,
fragmentNames2[j],
);
}
for (const fragmentName2 of fragmentNames2) {
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap1,
fragmentName2,
);
}

// (I) Then collect conflicts between the second collection of fields and
// those referenced by each fragment name associated with the first.
if (fragmentNames1.length !== 0) {
for (let i = 0; i < fragmentNames1.length; i++) {
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap2,
fragmentNames1[i],
);
}
for (const fragmentName1 of fragmentNames1) {
collectConflictsBetweenFieldsAndFragment(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fieldMap2,
fragmentName1,
);
}

// (J) Also collect conflicts between any fragment names by the first and
// fragment names by the second. This compares each item in the first set of
// names to each item in the second set of names.
for (let i = 0; i < fragmentNames1.length; i++) {
for (let j = 0; j < fragmentNames2.length; j++) {
for (const fragmentName1 of fragmentNames1) {
for (const fragmentName2 of fragmentNames2) {
collectConflictsBetweenFragments(
context,
conflicts,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
areMutuallyExclusive,
fragmentNames1[i],
fragmentNames2[j],
fragmentName1,
fragmentName2,
);
}
}
Expand Down Expand Up @@ -511,16 +510,16 @@ function collectConflictsBetween(
for (const [responseName, fields1] of Object.entries(fieldMap1)) {
const fields2 = fieldMap2[responseName];
if (fields2) {
for (let i = 0; i < fields1.length; i++) {
for (let j = 0; j < fields2.length; j++) {
for (const field1 of fields1) {
for (const field2 of fields2) {
const conflict = findConflict(
context,
cachedFieldsAndFragmentNames,
comparedFragmentPairs,
parentFieldsAreMutuallyExclusive,
responseName,
fields1[i],
fields2[j],
field1,
field2,
);
if (conflict) {
conflicts.push(conflict);
Expand Down
2 changes: 1 addition & 1 deletion src/validation/rules/SingleFieldSubscriptionsRule.ts
Expand Up @@ -61,7 +61,7 @@ export function SingleFieldSubscriptionsRule(
for (const fieldNodes of fields.values()) {
const field = fieldNodes[0];
const fieldName = field.name.value;
if (fieldName[0] === '_' && fieldName[1] === '_') {
if (fieldName.startsWith('__')) {
context.reportError(
new GraphQLError(
operationName != null
Expand Down