Skip to content

Commit

Permalink
Merge pull request #2294 from vuejs/cross-file-template-type-checking
Browse files Browse the repository at this point in the history
Fix #1596
  • Loading branch information
octref authored Sep 21, 2020
2 parents 8c12f6a + e17462c commit 73cc4de
Show file tree
Hide file tree
Showing 43 changed files with 843 additions and 322 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### 0.28.0

- Cross file template type checking - check that components are passed props with the correct types. #1596 and #2294.

### 0.27.3 | 2020-09-13 | [VSIX](https://marketplace.visualstudio.com/_apis/public/gallery/publishers/octref/vsextensions/vetur/0.27.3/vspackage)

- 🙌 Fix corner case when analyzing class component. Thanks to contribution from [@yoyo930021](https://github.com/yoyo930021). #2254 and #2260.
Expand Down
54 changes: 53 additions & 1 deletion docs/interpolation.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ export default {

## Prop Validation

*You can turn on/off this feature with `vetur.validation.templateProps`.*

Vetur will now validate HTML templates that uses child components. For example, given two children:

`Simple.vue`:
Expand Down Expand Up @@ -120,4 +122,54 @@ Vetur will show a warming for `<simple>` and an error for `<complex>`.
The rules are:

- When using [array props](https://vuejs.org/v2/guide/components-props.html#Prop-Types), show **warning** for missing props.
- When using [object prop validation](https://vuejs.org/v2/guide/components-props.html#Prop-Validation), show errors for missing `required` props.
- When using [object prop validation](https://vuejs.org/v2/guide/components-props.html#Prop-Validation), show errors for missing `required` props.

## Prop Type Validation

*You can turn on/off this feature with `vetur.validation.templateProps`.*

Vetur will now validate that the interpolation expression you pass to child component's props match the props signature. Consider this simple case:

`Child.vue`:

```vue
<template>
<div></div>
</template>
<script>
export default {
props: { str: String }
}
</script>
```

`Parent.vue`:

```vue
<template>
<test :str="num" />
</template>
<script>
import Test from './Test.vue'
export default {
components: { Test },
data() {
return {
num: 42
}
}
}
</script>
```

Vetur will generate a diagnostic error on `str` in `Parent.vue` template `:str="num"`, with a message that `type 'number' is not assignable to type 'string'`.

Supported:

- JS file with `export default {...}`
- TS file with `defineComponent` in Vue 3 or `Vue.extend` in Vue 2
- Prop Type: `foo: String`, `foo: { type: String }` or `foo: String as PropType<string>`
- This is useful in the case of `foo: Array`. If you are using JS, there's no way to say `foo is a string array`, however with TS you can use `foo: Array as PropType<string[]>`. Vetur will then check that the provided expression matches `string[]`.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@
"vetur.validation.templateProps": {
"type": "boolean",
"default": false,
"description": "Validate props usage in <template> region. Show error/warning for not passing declared props to child components."
"description": "Validate props usage in <template> region. Show error/warning for not passing declared props to child components and show error for passing wrongly typed interpolation expressions"
},
"vetur.validation.interpolation": {
"type": "boolean",
Expand Down
2 changes: 1 addition & 1 deletion server/src/embeddedSupport/languageModes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class LanguageModes {
}

/**
* Documents where everything outside `<script>~ is replaced with whitespace
* Documents where everything outside `<script>` is replaced with whitespace
*/
const scriptRegionDocuments = getLanguageModelCache(10, 60, document => {
const vueDocument = this.documentRegions.refreshAndGet(document);
Expand Down
82 changes: 78 additions & 4 deletions server/src/modes/script/componentInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,90 @@ function getProps(tsModule: T_TypeScript, defaultExportType: ts.Type, checker: t

function getPropValidatorInfo(
propertyValue: ts.Node | undefined
): { hasObjectValidator: boolean; required: boolean } {
if (!propertyValue || !tsModule.isObjectLiteralExpression(propertyValue)) {
): { hasObjectValidator: boolean; required: boolean; typeString?: string } {
if (!propertyValue) {
return { hasObjectValidator: false, required: true };
}

let typeString: string | undefined = undefined;
let typeDeclaration: ts.Identifier | ts.AsExpression | undefined = undefined;

/**
* case `foo: { type: String }`
* extract type value: `String`
*/
if (tsModule.isObjectLiteralExpression(propertyValue)) {
const propertyValueSymbol = checker.getTypeAtLocation(propertyValue).symbol;
const typeValue = propertyValueSymbol?.members?.get('type' as ts.__String)?.valueDeclaration;
if (typeValue && tsModule.isPropertyAssignment(typeValue)) {
if (tsModule.isIdentifier(typeValue.initializer) || tsModule.isAsExpression(typeValue.initializer)) {
typeDeclaration = typeValue.initializer;
}
}
} else {
/**
* case `foo: String`
* extract type value: `String`
*/
if (tsModule.isIdentifier(propertyValue) || tsModule.isAsExpression(propertyValue)) {
typeDeclaration = propertyValue;
}
}

if (typeDeclaration) {
/**
* `String` case
*
* Per https://vuejs.org/v2/guide/components-props.html#Type-Checks, handle:
*
* String
* Number
* Boolean
* Array
* Object
* Date
* Function
* Symbol
*/
if (tsModule.isIdentifier(typeDeclaration)) {
const vueTypeCheckConstructorToTSType: Record<string, string> = {
String: 'string',
Number: 'number',
Boolean: 'boolean',
Array: 'any[]',
Object: 'object',
Date: 'Date',
Function: 'Function',
Symbol: 'Symbol'
};
const vueTypeString = typeDeclaration.getText();
if (vueTypeCheckConstructorToTSType[vueTypeString]) {
typeString = vueTypeCheckConstructorToTSType[vueTypeString];
}
} else if (
/**
* `String as PropType<'a' | 'b'>` case
*/
tsModule.isAsExpression(typeDeclaration) &&
tsModule.isTypeReferenceNode(typeDeclaration.type) &&
typeDeclaration.type.typeName.getText() === 'PropType' &&
typeDeclaration.type.typeArguments &&
typeDeclaration.type.typeArguments[0]
) {
const extractedPropType = typeDeclaration.type.typeArguments[0];
typeString = extractedPropType.getText();
}
}

if (!propertyValue || !tsModule.isObjectLiteralExpression(propertyValue)) {
return { hasObjectValidator: false, required: true, typeString };
}

const propertyValueSymbol = checker.getTypeAtLocation(propertyValue).symbol;
const requiredValue = propertyValueSymbol?.members?.get('required' as ts.__String)?.valueDeclaration;
const defaultValue = propertyValueSymbol?.members?.get('default' as ts.__String)?.valueDeclaration;
if (!requiredValue && !defaultValue) {
return { hasObjectValidator: false, required: true };
return { hasObjectValidator: false, required: true, typeString };
}

const required = Boolean(
Expand All @@ -156,7 +230,7 @@ function getProps(tsModule: T_TypeScript, defaultExportType: ts.Type, checker: t
requiredValue?.initializer.kind === tsModule.SyntaxKind.TrueKeyword
);

return { hasObjectValidator: true, required };
return { hasObjectValidator: true, required, typeString };
}

function getClassProps(type: ts.Type) {
Expand Down
9 changes: 4 additions & 5 deletions server/src/modes/template/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class VueHTMLMode implements LanguageMode {
const vueDocuments = getLanguageModelCache<HTMLDocument>(10, 60, document => parseHTMLDocument(document));
const vueVersion = inferVueVersion(tsModule, workspacePath);
this.htmlMode = new HTMLMode(documentRegions, workspacePath, vueVersion, vueDocuments, vueInfoService);
this.vueInterpolationMode = new VueInterpolationMode(tsModule, serviceHost, vueDocuments);
this.vueInterpolationMode = new VueInterpolationMode(tsModule, serviceHost, vueDocuments, vueInfoService);
}
getId() {
return 'vue-html';
Expand Down Expand Up @@ -81,10 +81,9 @@ export class VueHTMLMode implements LanguageMode {
return this.vueInterpolationMode.findReferences(document, position);
}
findDefinition(document: TextDocument, position: Position) {
const interpolationDefinition = this.vueInterpolationMode.findDefinition(document, position);
return interpolationDefinition.length > 0
? interpolationDefinition
: this.htmlMode.findDefinition(document, position);
const htmlDefinition = this.htmlMode.findDefinition(document, position);

return htmlDefinition.length > 0 ? htmlDefinition : this.vueInterpolationMode.findDefinition(document, position);
}
getFoldingRanges(document: TextDocument) {
return this.htmlMode.getFoldingRanges(document);
Expand Down
77 changes: 49 additions & 28 deletions server/src/modes/template/interpolationMode.ts
Original file line number Diff line number Diff line change
@@ -1,42 +1,44 @@
import { LanguageMode } from '../../embeddedSupport/languageModes';
import * as _ from 'lodash';
import * as ts from 'typescript';
import {
CompletionItem,
CompletionList,
Definition,
Diagnostic,
TextDocument,
DiagnosticSeverity,
Position,
Location,
MarkedString,
MarkupContent,
Position,
Range,
Location,
Definition,
CompletionList,
TextEdit,
CompletionItem,
MarkupContent
TextDocument,
TextEdit
} from 'vscode-languageserver-types';
import { IServiceHost } from '../../services/typescriptService/serviceHost';
import { languageServiceIncludesFile } from '../script/javascript';
import { getFileFsPath } from '../../utils/paths';
import { mapBackRange, mapFromPositionToOffset } from '../../services/typescriptService/sourceMap';
import { URI } from 'vscode-uri';
import * as ts from 'typescript';
import { VLSFullConfig } from '../../config';
import { LanguageModelCache } from '../../embeddedSupport/languageModelCache';
import { LanguageMode } from '../../embeddedSupport/languageModes';
import { T_TypeScript } from '../../services/dependencyService';
import * as _ from 'lodash';
import { IServiceHost } from '../../services/typescriptService/serviceHost';
import { mapBackRange, mapFromPositionToOffset } from '../../services/typescriptService/sourceMap';
import { createTemplateDiagnosticFilter } from '../../services/typescriptService/templateDiagnosticFilter';
import { NULL_COMPLETION } from '../nullMode';
import { toCompletionItemKind } from '../../services/typescriptService/util';
import { LanguageModelCache } from '../../embeddedSupport/languageModelCache';
import { VueInfoService } from '../../services/vueInfoService';
import { getFileFsPath } from '../../utils/paths';
import { NULL_COMPLETION } from '../nullMode';
import { languageServiceIncludesFile } from '../script/javascript';
import * as Previewer from '../script/previewer';
import { HTMLDocument } from './parser/htmlParser';
import { isInsideInterpolation } from './services/isInsideInterpolation';
import * as Previewer from '../script/previewer';
import { VLSFullConfig } from '../../config';

export class VueInterpolationMode implements LanguageMode {
private config: VLSFullConfig;

constructor(
private tsModule: T_TypeScript,
private serviceHost: IServiceHost,
private vueDocuments: LanguageModelCache<HTMLDocument>
private vueDocuments: LanguageModelCache<HTMLDocument>,
private vueInfoService?: VueInfoService
) {}

getId() {
Expand Down Expand Up @@ -67,7 +69,15 @@ export class VueInterpolationMode implements LanguageMode {
document.getText()
);

const { templateService, templateSourceMap } = this.serviceHost.updateCurrentVirtualVueTextDocument(templateDoc);
const childComponents = this.config.vetur.validation.templateProps
? this.vueInfoService && this.vueInfoService.getInfo(document)?.componentInfo.childComponents
: [];

const { templateService, templateSourceMap } = this.serviceHost.updateCurrentVirtualVueTextDocument(
templateDoc,
childComponents
);

if (!languageServiceIncludesFile(templateService, templateDoc.uri)) {
return [];
}
Expand Down Expand Up @@ -135,16 +145,27 @@ export class VueInterpolationMode implements LanguageMode {
const mappedOffset = mapFromPositionToOffset(templateDoc, completionPos, templateSourceMap);
const templateFileFsPath = getFileFsPath(templateDoc.uri);

const completions = templateService.getCompletionsAtPosition(templateFileFsPath, mappedOffset, {
includeCompletionsWithInsertText: true,
includeCompletionsForModuleExports: false
});
/**
* A lot of times interpolation expressions aren't valid
* TODO: Make sure interpolation expression, even incomplete, can generate incomplete
* TS files that can be fed into language service
*/
let completions: ts.WithMetadata<ts.CompletionInfo> | undefined;
try {
completions = templateService.getCompletionsAtPosition(templateFileFsPath, mappedOffset, {
includeCompletionsWithInsertText: true,
includeCompletionsForModuleExports: false
});
} catch (err) {
console.log('Interpolation completion failed');
console.error(err.toString());
}

if (!completions) {
return NULL_COMPLETION;
}

const tsItems = completions.entries.map((entry, index) => {
const tsItems = completions!.entries.map((entry, index) => {
return {
uri: templateDoc.uri,
position,
Expand Down Expand Up @@ -334,7 +355,7 @@ export class VueInterpolationMode implements LanguageMode {
: convertRange(definitionTargetDoc, r.textSpan);

definitionResults.push({
uri: URI.file(definitionTargetDoc.uri).toString(),
uri: definitionTargetDoc.uri,
range
});
}
Expand Down Expand Up @@ -382,7 +403,7 @@ export class VueInterpolationMode implements LanguageMode {
: convertRange(referenceTargetDoc, r.textSpan);

referenceResults.push({
uri: URI.file(referenceTargetDoc.uri).toString(),
uri: referenceTargetDoc.uri,
range
});
}
Expand Down
14 changes: 9 additions & 5 deletions server/src/modes/template/services/htmlDefinition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,29 @@ export function findDefinition(
position: Position,
htmlDocument: HTMLDocument,
vueFileInfo?: VueFileInfo
): Definition {
): Location[] {
const offset = document.offsetAt(position);
const node = htmlDocument.findNodeAt(offset);
if (!node || !node.tag) {
return [];
}

function getTagDefinition(tag: string, range: Range, open: boolean): Definition {
function getTagDefinition(tag: string, range: Range, open: boolean): Location[] {
if (vueFileInfo && vueFileInfo.componentInfo.childComponents) {
for (const cc of vueFileInfo.componentInfo.childComponents) {
if (![tag, tag.toLowerCase(), kebabCase(tag)].includes(cc.name)) { continue; }
if (!cc.definition) { continue; }
if (![tag, tag.toLowerCase(), kebabCase(tag)].includes(cc.name)) {
continue;
}
if (!cc.definition) {
continue;
}

const loc: Location = {
uri: URI.file(cc.definition.path).toString(),
// Todo: Resolve actual default export range
range: Range.create(0, 0, 0, 0)
};
return loc;
return [loc];
}
}
return [];
Expand Down
Loading

0 comments on commit 73cc4de

Please sign in to comment.