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

Put annotations of removed pieces of schema on new schema #1414

Merged
merged 3 commits into from
Apr 10, 2020
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### vNEXT

- **github**: put annotations of removed parts of schema on new schema [#1414](https://github.com/kamilkisiela/graphql-inspector/issues/1414)

### v1.30.4

- **github**: summary page with details (like in Github Action)
Expand Down
2 changes: 1 addition & 1 deletion action/index.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions packages/github/__tests__/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ test('annotations should match lines in schema file', async () => {
});

// Field 'modifiedAt' was removed from object type 'Post'
expect(getPrintedLine(sources.old, action.annotations![0].start_line)).toBe(
'modifiedAt: String',
expect(getPrintedLine(sources.new, action.annotations![0].start_line)).toBe(
'type Post {',
);

// Field 'Post.createdAt' changed type from 'String' to 'String!'
Expand Down Expand Up @@ -158,8 +158,8 @@ test('should work with comments and descriptions', async () => {
'type User {',
);
// Field 'version' was removed from object type 'Meta'
expect(getPrintedLine(sources.old, action.annotations![1].start_line)).toBe(
'version: String!',
expect(getPrintedLine(sources.new, action.annotations![1].start_line)).toBe(
'type Meta {',
);
// Field 'Meta.name' changed type from 'String!' to 'String'
expect(getPrintedLine(sources.new, action.annotations![2].start_line)).toBe(
Expand Down
66 changes: 66 additions & 0 deletions packages/github/__tests__/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,69 @@ test('location of a RootType.Field', () => {

expect(printedLine(source, line)).toMatch('user(id: ID!): User');
});

test('Type.Field.Arg: non-existing Arg should point to Type.Field ', () => {
const {line} = getLocationByPath({
source,
path: 'Query.user.nonExisting',
});

expect(printedLine(source, line)).toMatch('user(id: ID!): User');
});

test('Type.Field.Arg: non-existing Field should point to Type ', () => {
const {line} = getLocationByPath({
source,
path: 'Query.nonExisting.id',
});

expect(printedLine(source, line)).toMatch('type Query {');
});

test('Type.Field.Arg: non-existing Type should point to first line ', () => {
const {line} = getLocationByPath({
source,
path: 'NonExisting.user.id',
});

expect(printedLine(source, line)).toMatch(/^\s*$/);
});

test('Directive.Field.Arg: non-existing Type should point to first line ', () => {
const {line} = getLocationByPath({
source,
path: 'NonExisting.user.id',
});

expect(printedLine(source, line)).toMatch(/^\s*$/);
});

test('Enum.Value: non-existing Value should point to Enum', () => {
const testSource = new Source(/* GraphQL */`
enum MyEnum {
Foo
Bar
}
`);
const {line} = getLocationByPath({
source: testSource,
path: 'MyEnum.Nonexisting',
});

expect(printedLine(testSource, line)).toMatch('enum MyEnum {');
});

test('Enum.Value: Value should point to Value', () => {
const testSource = new Source(/* GraphQL */`
enum MyEnum {
Foo
Bar
}
`);
const {line} = getLocationByPath({
source: testSource,
path: 'MyEnum.Bar',
});

expect(printedLine(testSource, line)).toMatch('Bar');
});
11 changes: 3 additions & 8 deletions packages/github/src/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export async function diff({
}

const annotations = await Promise.all(
changes.map((change) => annotate({path, change, sources})),
changes.map((change) => annotate({path, change, source: sources.new})),
);
let conclusion: CheckConclusion = CheckConclusion.Success;

Expand Down Expand Up @@ -65,18 +65,13 @@ const levelMap = {
function annotate({
path,
change,
sources,
source,
}: {
path: string;
change: Change;
sources: {
old: Source;
new: Source;
};
source: Source;
}): Annotation {
const level = change.criticality.level;
const useOld = change.type.endsWith('_REMOVED');
const source = useOld ? sources.old : sources.new;
const loc = change.path
? getLocationByPath({path: change.path, source})
: {line: 1, column: 1};
Expand Down
93 changes: 59 additions & 34 deletions packages/github/src/location.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export function getLocationByPath({
}
}

return resolveNodeSourceLocation(source, resolvedNode);;
return resolveNodeSourceLocation(source, resolvedNode);
}

type Node =
Expand All @@ -115,24 +115,51 @@ function resolveUnionTypeDefinitionNode(
return definition;
}

function resolveInterfaceTypeDefinition(
function resolveArgument(argName: string, field: FieldDefinitionNode) {
const arg = field.arguments?.find((a) => a.name.value === argName);

return arg || field;
}

function resolveFieldDefinition(
path: string[],
definition: InterfaceTypeDefinitionNode,
definition:
| InterfaceTypeDefinitionNode
| InputObjectTypeDefinitionNode
| ObjectTypeDefinitionNode,
): Node {
const [fieldName, argName] = path;

if (!fieldName) {
console.log(definition);
return definition;
}
const fieldIndex =
definition.fields?.findIndex(
(f: InputValueDefinitionNode | FieldDefinitionNode) =>
f.name.value === fieldName,
);

if (typeof fieldIndex === 'number' && fieldIndex > -1) {
const field = definition.fields![fieldIndex];

const field = definition.fields!.find((f) => f.name.value === fieldName)!;
if (field.kind !== Kind.INPUT_VALUE_DEFINITION && argName) {
return resolveArgument(argName, field);
}

if (!argName) {
return field;
}

return field.arguments!.find((arg) => arg.name.value === argName);
return definition;
}

function resolveInterfaceTypeDefinition(
path: string[],
definition: InterfaceTypeDefinitionNode,
): Node {
const [fieldName, argName] = path;

if (fieldName) {
return resolveFieldDefinition([fieldName, argName], definition);
}

return definition;
}

function resolveInputObjectTypeDefinition(
Expand All @@ -141,13 +168,11 @@ function resolveInputObjectTypeDefinition(
): Node {
const [fieldName] = path;

if (!fieldName) {
return definition;
if (fieldName) {
return resolveFieldDefinition([fieldName], definition);
}

const field = definition.fields!.find((field) => field.name.value)!;

return field;
return definition;
}

function resolveEnumTypeDefinition(
Expand All @@ -156,11 +181,15 @@ function resolveEnumTypeDefinition(
): Node {
const [valueName] = path;

if (!valueName) {
return definition;
if (definition.values && valueName) {
const value = definition.values.find((val) => val.name.value === valueName);

if (value) {
return value;
}
}

return definition.values!.find((val) => val.name.value === valueName);
return definition;
}

function resolveObjectTypeDefinition(
Expand All @@ -169,17 +198,11 @@ function resolveObjectTypeDefinition(
): Node {
const [fieldName, argName] = path;

if (!fieldName) {
return definition;
if (fieldName) {
return resolveFieldDefinition([fieldName, argName], definition);
}

const field = definition.fields!.find((f) => f.name.value === fieldName)!;

if (!argName) {
return field;
}

return field.arguments!.find((arg) => arg.name.value === argName);
return definition;
}

function resolveDirectiveDefinition(
Expand All @@ -188,15 +211,17 @@ function resolveDirectiveDefinition(
): Node {
const [argName] = path;

if (!argName) {
return defininition;
}
if (defininition.arguments && argName) {
const arg = defininition.arguments.find(
(arg) => arg.name.value === argName,
);

const arg = defininition.arguments!.find(
(arg) => arg.name.value === argName,
)!;
if (arg) {
return arg;
}
}

return arg;
return defininition;
}

function resolveNodeSourceLocation(source: Source, node: Node): SourceLocation {
Expand Down