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

enable lonely-if/unicorn/lonely-if rules #2936

Merged
merged 2 commits into from
Dec 8, 2022
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/real-doors-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'codemirror-graphql': patch
'@graphiql/toolkit': patch
'graphql-language-service': patch
'vscode-graphql-execution': patch
---

enable `lonely-if`/`unicorn/lonely-if` rules
5 changes: 3 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ module.exports = {
'no-bitwise': 1,
'no-continue': 0,
'no-inline-comments': 0,
'no-lonely-if': 0,
'no-lonely-if': 'error',
'unicorn/no-lonely-if': 'error',
'no-mixed-operators': 0,
'no-negated-condition': 'error',
'no-nested-ternary': 0,
Expand Down Expand Up @@ -264,7 +265,7 @@ module.exports = {
'jest/no-conditional-expect': 0,
},

plugins: ['import', '@typescript-eslint'],
plugins: ['import', '@typescript-eslint', 'unicorn'],

overrides: [
// Cypress plugin, global, etc only for cypress directory
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
"eslint-plugin-jest": "^27.1.6",
"eslint-plugin-react": "^7.31.11",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-unicorn": "^45.0.1",
"execa": "^6.0.0",
"express": "^4.17.3",
"fetch-mock": "6.5.2",
Expand Down
8 changes: 3 additions & 5 deletions packages/codemirror-graphql/src/__tests__/lint-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,9 @@ function printLintErrors(queryString: string, configOverrides = {}) {

return new Promise<any[]>(resolve => {
editor.state.lint.options.onUpdateLinting = (errors: any[]) => {
if (errors?.[0]) {
if (!errors[0].message.match('Unexpected EOF')) {
resolve(errors);
return;
}
if (errors?.[0] && !errors[0].message.match('Unexpected EOF')) {
resolve(errors);
return;
}
resolve([]);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ function printLintErrors(query: Maybe<string>, variables: string) {
editor.state.lint.options.onUpdateLinting = (
errors: CodeMirror.Annotation[],
) => {
if (errors?.[0]) {
if (!errors[0].message?.match('Unexpected EOF')) {
resolve(errors);
return;
}
if (errors?.[0] && !errors[0].message?.match('Unexpected EOF')) {
resolve(errors);
return;
}
resolve([]);
};
Expand Down
31 changes: 16 additions & 15 deletions packages/codemirror-graphql/src/variables/hint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,21 +118,22 @@ function getVariablesHint(
}

// Input Object fields
if (kind === 'ObjectValue' || (kind === 'ObjectField' && step === 0)) {
if (typeInfo.fields) {
const inputFields = Object.keys(typeInfo.fields).map(
fieldName => typeInfo.fields![fieldName],
);
return hintList(
cur,
token,
inputFields.map(field => ({
text: `"${field.name}": `,
type: field.type,
description: field.description,
})),
);
}
if (
(kind === 'ObjectValue' || (kind === 'ObjectField' && step === 0)) &&
typeInfo.fields
) {
const inputFields = Object.keys(typeInfo.fields).map(
fieldName => typeInfo.fields![fieldName],
);
return hintList(
cur,
token,
inputFields.map(field => ({
text: `"${field.name}": `,
type: field.type,
description: field.description,
})),
);
}

// Input values.
Expand Down
35 changes: 18 additions & 17 deletions packages/codemirror-graphql/src/variables/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,15 @@ function validateValue(
// Look for missing non-nullable fields.
Object.keys(type.getFields()).forEach(fieldName => {
const field = type.getFields()[fieldName];
if (!providedFields[fieldName]) {
if (field.type instanceof GraphQLNonNull && !field.defaultValue) {
fieldErrors.push([
valueAST,
`Object of type "${type}" is missing required field "${fieldName}".`,
]);
}
if (
!providedFields[fieldName] &&
field.type instanceof GraphQLNonNull &&
!field.defaultValue
) {
fieldErrors.push([
valueAST,
`Object of type "${type}" is missing required field "${fieldName}".`,
]);
}
});

Expand All @@ -200,16 +202,15 @@ function validateValue(
}

// Validate enums and custom scalars.
if (type instanceof GraphQLEnumType || type instanceof GraphQLScalarType) {
if (
(valueAST.kind !== 'String' &&
valueAST.kind !== 'Number' &&
valueAST.kind !== 'Boolean' &&
valueAST.kind !== 'Null') ||
isNullish(type.parseValue(valueAST.value))
) {
return [[valueAST, `Expected value of type "${type}".`]];
}
if (
(type instanceof GraphQLEnumType || type instanceof GraphQLScalarType) &&
((valueAST.kind !== 'String' &&
valueAST.kind !== 'Number' &&
valueAST.kind !== 'Boolean' &&
valueAST.kind !== 'Null') ||
isNullish(type.parseValue(valueAST.value)))
) {
return [[valueAST, `Expected value of type "${type}".`]];
}

return [];
Expand Down
16 changes: 6 additions & 10 deletions packages/graphiql-toolkit/src/create-fetcher/lib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,8 @@ export const isSubscriptionWithName = (
let isSubscription = false;
visit(document, {
OperationDefinition(node) {
if (name === node.name?.value) {
if (node.operation === 'subscription') {
isSubscription = true;
}
if (name === node.name?.value && node.operation === 'subscription') {
isSubscription = true;
}
},
});
Expand Down Expand Up @@ -87,12 +85,10 @@ export const createWebsocketsFetcherFromUrl = (
});
return createWebsocketsFetcherFromClient(wsClient);
} catch (err) {
if (errorHasCode(err)) {
if (err.code === 'MODULE_NOT_FOUND') {
throw new Error(
"You need to install the 'graphql-ws' package to use websockets when passing a 'subscriptionUrl'",
);
}
if (errorHasCode(err) && err.code === 'MODULE_NOT_FOUND') {
throw new Error(
"You need to install the 'graphql-ws' package to use websockets when passing a 'subscriptionUrl'",
);
}
// eslint-disable-next-line no-console
console.error(`Error creating websocket client for ${url}`, err);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,26 +330,25 @@ export function getAutocompleteSuggestions(

// Input Object fields
if (
kind === RuleKinds.OBJECT_VALUE ||
(kind === RuleKinds.OBJECT_FIELD && step === 0)
(kind === RuleKinds.OBJECT_VALUE ||
(kind === RuleKinds.OBJECT_FIELD && step === 0)) &&
typeInfo.objectFieldDefs
) {
if (typeInfo.objectFieldDefs) {
const objectFields = objectValues(typeInfo.objectFieldDefs);
const completionKind =
kind === RuleKinds.OBJECT_VALUE
? CompletionItemKind.Value
: CompletionItemKind.Field;
return hintList(
token,
objectFields.map(field => ({
label: field.name,
detail: String(field.type),
documentation: field.description ?? undefined,
kind: completionKind,
type: field.type,
})),
);
}
const objectFields = objectValues(typeInfo.objectFieldDefs);
const completionKind =
kind === RuleKinds.OBJECT_VALUE
? CompletionItemKind.Value
: CompletionItemKind.Field;
return hintList(
token,
objectFields.map(field => ({
label: field.name,
detail: String(field.type),
documentation: field.description ?? undefined,
kind: completionKind,
type: field.type,
})),
);
}

// Input values: Enum and Boolean
Expand Down Expand Up @@ -871,21 +870,19 @@ export function getVariableCompletions(
}
}

if (variableName && variableType) {
if (!definitions[variableName]) {
// append `$` if the `token.string` is not already `$`
if (variableName && variableType && !definitions[variableName]) {
// append `$` if the `token.string` is not already `$`

definitions[variableName] = {
detail: variableType.toString(),
insertText: token.string === '$' ? variableName : '$' + variableName,
label: variableName, // keep label the same for `codemirror-graphql`
type: variableType,
kind: CompletionItemKind.Variable,
} as CompletionItem;
definitions[variableName] = {
detail: variableType.toString(),
insertText: token.string === '$' ? variableName : '$' + variableName,
label: variableName, // keep label the same for `codemirror-graphql`
type: variableType,
kind: CompletionItemKind.Variable,
} as CompletionItem;

variableName = null;
variableType = null;
}
variableName = null;
variableType = null;
}
});

Expand Down Expand Up @@ -976,13 +973,14 @@ export function getTokenAtPosition(
let stateAtCursor = null;
let stringAtCursor = null;
const token = runOnlineParser(queryText, (stream, state, style, index) => {
if (index === cursor.line) {
if (stream.getCurrentPosition() >= cursor.character) {
styleAtCursor = style;
stateAtCursor = { ...state };
stringAtCursor = stream.current();
return 'BREAK';
}
if (
index === cursor.line &&
stream.getCurrentPosition() >= cursor.character
) {
styleAtCursor = style;
stateAtCursor = { ...state };
stringAtCursor = stream.current();
return 'BREAK';
}
});

Expand Down
23 changes: 11 additions & 12 deletions packages/graphql-language-service/src/parser/CharacterStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,24 +118,23 @@ export default class CharacterStream implements CharacterStreamInterface {
token = match?.[0];
}

if (match != null) {
if (
typeof pattern === 'string' ||
if (
match != null &&
(typeof pattern === 'string' ||
(match instanceof Array &&
// String.match returns 'index' property, which flow fails to detect
// for some reason. The below is a workaround, but an easier solution
// is just checking if `match.index === 0`
this._sourceText.startsWith(match[0], this._pos))
) {
if (consume) {
this._start = this._pos;
// eslint-disable-next-line @typescript-eslint/prefer-optional-chain -- otherwise has type issue
if (token && token.length) {
this._pos += token.length;
}
this._sourceText.startsWith(match[0], this._pos)))
) {
if (consume) {
this._start = this._pos;
// eslint-disable-next-line @typescript-eslint/prefer-optional-chain -- otherwise has type issue
if (token && token.length) {
this._pos += token.length;
}
return match;
}
return match;
}

// No match available.
Expand Down
13 changes: 6 additions & 7 deletions packages/graphql-language-service/src/parser/onlineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,12 @@ function getToken(
const levels = (state.levels = (state.levels || []).slice(0, -1));
// FIXME
// what if state.indentLevel === 0?
if (state.indentLevel) {
if (
levels.length > 0 &&
levels[levels.length - 1] < state.indentLevel
) {
state.indentLevel = levels[levels.length - 1];
}
if (
state.indentLevel &&
levels.length > 0 &&
levels[levels.length - 1] < state.indentLevel
) {
state.indentLevel = levels[levels.length - 1];
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ export function collectVariables(
) as GraphQLInputType;
if (inputType) {
variableToType[variable.name.value] = inputType;
} else if (type.kind === Kind.NAMED_TYPE) {
} else if (
type.kind === Kind.NAMED_TYPE &&
// in the experimental stream defer branch we are using, it seems typeFromAST() doesn't recognize Floats?
if (type.name.value === 'Float') {
variableToType[variable.name.value] = GraphQLFloat;
}
type.name.value === 'Float'
) {
variableToType[variable.name.value] = GraphQLFloat;
}
});
}
Expand Down
20 changes: 9 additions & 11 deletions packages/vscode-graphql-execution/src/helpers/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,18 +155,16 @@ export class NetworkHelper {
urqlClient.subscription(parsedOperation, variables),
subscribe(subscriber),
);
} else if (operation === 'query') {
pipe(
urqlClient.query(parsedOperation, variables),
subscribe(subscriber),
);
} else {
if (operation === 'query') {
pipe(
urqlClient.query(parsedOperation, variables),
subscribe(subscriber),
);
} else {
pipe(
urqlClient.mutation(parsedOperation, variables),
subscribe(subscriber),
);
}
pipe(
urqlClient.mutation(parsedOperation, variables),
subscribe(subscriber),
);
}
} catch (err) {
this.outputChannel.appendLine(`error executing operation:\n${err}`);
Expand Down
Loading