Skip to content

Commit

Permalink
feat: implement quick fix for field type check
Browse files Browse the repository at this point in the history
  • Loading branch information
Yogu committed Aug 7, 2024
1 parent d55a201 commit 3eb4dbe
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 4 deletions.
43 changes: 42 additions & 1 deletion spec/model/compatibility-check/check-field-type.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,29 @@
import gql from 'graphql-tag';
import { expectSingleCompatibilityIssue } from '../implementation/validation-utils';
import {
expectQuickFix,
expectSingleCompatibilityIssue,
expectToBeValid,
} from '../implementation/validation-utils';
import { runCheck } from './utils';

describe('checkModel', () => {
describe('field type', () => {
it('accepts a correct type', () => {
const result = runCheck(
gql`
type Test @rootEntity @modules(in: "module1") {
field: String @modules(all: true)
}
`,
gql`
type Test @rootEntity {
field: String
}
`,
);
expectToBeValid(result);
});

it('rejects if a field has the wrong type', () => {
const result = runCheck(
gql`
Expand All @@ -21,6 +41,13 @@ describe('checkModel', () => {
result,
'Field "Test.field" needs to be of type "String" (required by module "module1").',
);
expectQuickFix(
result,
'Change type to "String"',
`type Test @rootEntity {
field: String
}`,
);
});

it('rejects if a field should be a list', () => {
Expand All @@ -40,6 +67,13 @@ describe('checkModel', () => {
result,
'Field "Test.field" needs to be a list (required by module "module1").',
);
expectQuickFix(
result,
'Change type to "[String]"',
`type Test @rootEntity {
field: [String]
}`,
);
});

it('rejects if a field wrongly is a list', () => {
Expand All @@ -59,6 +93,13 @@ describe('checkModel', () => {
result,
'Field "Test.field" should not be a list (required by module "module1").',
);
expectQuickFix(
result,
'Change type to "String"',
`type Test @rootEntity {
field: String
}`,
);
});
});
});
22 changes: 22 additions & 0 deletions spec/model/implementation/validation-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ValidationContext,
} from '../../../src/model/validation/validation-context';
import { Project } from '../../../src/project/project';
import { applyChangeSet } from '../../../core-exports';

type Validatable = ModelComponent | ValidationResult | Project;

Expand Down Expand Up @@ -61,3 +62,24 @@ export function expectSingleMessage(component: Validatable, errorPart: string, s
expect(message.message).to.equal(errorPart);
expect(message.severity).to.equal(severity);
}

export function expectQuickFix(
component: Validatable,
expectedDescription: string,
expectedBody: string,
) {
const result = validate(component);
const quickfixes = result.messages.flatMap((m) => m.quickFixes);
expect(quickfixes.map((q) => q.description)).to.include(expectedDescription);
const matchingQuickfixes = quickfixes.filter((q) => q.description);
expect(matchingQuickfixes).to.have.a.lengthOf(1);
const quickfix = matchingQuickfixes[0];
const changeSet = quickfix.getChangeSet();
expect(changeSet.changes).not.to.be.empty;
// dirty hack that works because our quickfix tests only use one file
const project = new Project([changeSet.changes[0].location.source]);
const newProject = applyChangeSet(project, changeSet);
expectToBeValid(newProject); // expect the fix to acutally fix the issues
expect(newProject.sources).to.have.a.lengthOf(1);
expect(newProject.sources[0].body).to.equal(expectedBody);
}
23 changes: 20 additions & 3 deletions src/model/compatibility-check/check-field-type.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { ChangeSet, TextChange } from '../change-set/change-set';
import { Field } from '../implementation/field';
import { QuickFix } from '../validation';
import { ValidationMessage } from '../validation/message';
import { ValidationContext } from '../validation/validation-context';
import { getRequiredBySuffix } from './describe-module-specification';
Expand All @@ -11,16 +13,29 @@ export function checkFieldType(
baselineField: Field,
context: ValidationContext,
) {
const quickFixes: QuickFix[] = [];
const expectedType = baselineField.isList
? '[' + baselineField.type.name + ']'
: baselineField.type.name;
if (fieldToCheck.astNode?.type.loc) {
quickFixes.push(
new QuickFix({
description: `Change type to "${expectedType}"`,
changeSet: new ChangeSet([
new TextChange(fieldToCheck.astNode.type.loc, expectedType),
]),
}),
);
}

if (fieldToCheck.type.name !== baselineField.type.name) {
const expectedType = baselineField.isList
? '[' + baselineField.type.name + ']'
: baselineField.type.name;
context.addMessage(
ValidationMessage.compatibilityIssue(
`Field "${baselineField.declaringType.name}.${
baselineField.name
}" needs to be of type "${expectedType}"${getRequiredBySuffix(baselineField)}.`,
fieldToCheck.astNode?.type,
{ quickFixes },
),
);
} else if (fieldToCheck.isList && !baselineField.isList) {
Expand All @@ -30,6 +45,7 @@ export function checkFieldType(
baselineField.name
}" should not be a list${getRequiredBySuffix(baselineField)}.`,
fieldToCheck.astNode?.type,
{ quickFixes },
),
);
} else if (!fieldToCheck.isList && baselineField.isList) {
Expand All @@ -39,6 +55,7 @@ export function checkFieldType(
baselineField.name
}" needs to be a list${getRequiredBySuffix(baselineField)}.`,
fieldToCheck.astNode?.type,
{ quickFixes },
),
);
}
Expand Down

0 comments on commit 3eb4dbe

Please sign in to comment.