Skip to content

Fix #1596 #2294

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

Merged
merged 7 commits into from
Sep 21, 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
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