diff --git a/README.md b/README.md index ba5e7b56..c0b163f5 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,10 @@ rules/tags added to `warn_list` (see [Ansible Lint Documentation](https://ansible-lint.readthedocs.io/en/latest/configuring.html)) are shown as warnings instead. +> If you also install `yamllint`, `ansible-lint` will detect it and incorporate +> into the linting process. Any findings reported by `yamllint` will be exposed +> in VSCode as errors/warnings. + ### Smart autocompletion ![Autocompletion](images/smart-completions.gif) @@ -92,7 +96,8 @@ holding `ctrl`/`cmd`. ## Requirements - [Ansible 2.9+](https://docs.ansible.com/ansible/latest/index.html) - [Ansible Lint](https://ansible-lint.readthedocs.io/en/latest/) (required, - unless you disable linter support; install without `yamllint`) + unless you disable linter support) +- [yamllint](https://yamllint.readthedocs.io/en/stable/) (optional) For Windows users, this extension works perfectly well with extensions such as `Remote - WSL` and `Remote - Containers`. diff --git a/src/providers/completionProvider.ts b/src/providers/completionProvider.ts index 15cd0147..9cf0bef7 100644 --- a/src/providers/completionProvider.ts +++ b/src/providers/completionProvider.ts @@ -6,7 +6,6 @@ import { TextEdit, } from 'vscode-languageserver'; import { Position, TextDocument } from 'vscode-languageserver-textdocument'; -import { parseAllDocuments } from 'yaml'; import { Node, Pair, Scalar, YAMLMap } from 'yaml/types'; import { IOption } from '../interfaces/module'; import { WorkspaceFolderContext } from '../services/workspaceManager'; @@ -24,11 +23,13 @@ import { findProvidedModule, getDeclaredCollections, getPathAt, + getOrigRange, getYamlMapKeys, isBlockParam, isPlayParam, isRoleParam, isTaskParam, + parseAllDocuments, } from '../utils/yaml'; const priorityMap = { @@ -293,9 +294,10 @@ function getKeywordCompletion( * the node has range information and is a string scalar. */ function getNodeRange(node: Node, document: TextDocument): Range | undefined { - if (node.range && node instanceof Scalar && typeof node.value === 'string') { - const start = node.range[0]; - let end = node.range[1]; + const range = getOrigRange(node); + if (range && node instanceof Scalar && typeof node.value === 'string') { + const start = range[0]; + let end = range[1]; // compensate for `_:` if (node.value.includes('_:')) { end -= 2; diff --git a/src/providers/definitionProvider.ts b/src/providers/definitionProvider.ts index cae248b3..3161fcc6 100644 --- a/src/providers/definitionProvider.ts +++ b/src/providers/definitionProvider.ts @@ -1,10 +1,15 @@ import { DefinitionLink, Range } from 'vscode-languageserver'; import { Position, TextDocument } from 'vscode-languageserver-textdocument'; -import { parseAllDocuments } from 'yaml'; import { Scalar } from 'yaml/types'; import { DocsLibrary } from '../services/docsLibrary'; import { toLspRange } from '../utils/misc'; -import { AncestryBuilder, getPathAt, isTaskParam } from '../utils/yaml'; +import { + AncestryBuilder, + getOrigRange, + getPathAt, + isTaskParam, + parseAllDocuments, +} from '../utils/yaml'; export async function getDefinition( document: TextDocument, @@ -26,11 +31,12 @@ export async function getDefinition( document.uri ); if (module) { + const range = getOrigRange(node); return [ { targetUri: module.source, - originSelectionRange: node.range - ? toLspRange(node.range, document) + originSelectionRange: range + ? toLspRange(range, document) : undefined, targetRange: Range.create( module.sourceLineRange[0], diff --git a/src/providers/hoverProvider.ts b/src/providers/hoverProvider.ts index 71dfec7d..f5914c64 100644 --- a/src/providers/hoverProvider.ts +++ b/src/providers/hoverProvider.ts @@ -1,6 +1,5 @@ import { Hover, MarkupContent, MarkupKind } from 'vscode-languageserver'; import { Position, TextDocument } from 'vscode-languageserver-textdocument'; -import { parseAllDocuments } from 'yaml'; import { Scalar, YAMLMap } from 'yaml/types'; import { DocsLibrary } from '../services/docsLibrary'; import { @@ -19,11 +18,13 @@ import { toLspRange } from '../utils/misc'; import { AncestryBuilder, findProvidedModule, + getOrigRange, getPathAt, isBlockParam, isPlayParam, isRoleParam, isTaskParam, + parseAllDocuments, } from '../utils/yaml'; export async function doHover( @@ -60,13 +61,14 @@ export async function doHover( path, document.uri ); + const range = getOrigRange(node); if (module && module.documentation) { return { contents: formatModule( module.documentation, docsLibrary.getModuleRoute(hitFqcn || node.value) ), - range: node.range ? toLspRange(node.range, document) : undefined, + range: range ? toLspRange(range, document) : undefined, }; } else if (hitFqcn) { // check for tombstones @@ -74,9 +76,7 @@ export async function doHover( if (route) { return { contents: formatTombstone(route), - range: node.range - ? toLspRange(node.range, document) - : undefined, + range: range ? toLspRange(range, document) : undefined, }; } } @@ -136,9 +136,10 @@ function getKeywordHover( } : keywordDocumentation; if (markupDoc) { + const range = getOrigRange(node); return { contents: markupDoc, - range: node.range ? toLspRange(node.range, document) : undefined, + range: range ? toLspRange(range, document) : undefined, }; } else return null; } diff --git a/src/providers/semanticTokenProvider.ts b/src/providers/semanticTokenProvider.ts index e97df193..56689945 100644 --- a/src/providers/semanticTokenProvider.ts +++ b/src/providers/semanticTokenProvider.ts @@ -5,7 +5,6 @@ import { SemanticTokenTypes, } from 'vscode-languageserver'; import { TextDocument } from 'vscode-languageserver-textdocument'; -import { parseAllDocuments } from 'yaml'; import { Node, Pair, Scalar, YAMLMap, YAMLSeq } from 'yaml/types'; import { IModuleMetadata } from '../interfaces/module'; import { DocsLibrary } from '../services/docsLibrary'; @@ -17,10 +16,12 @@ import { } from '../utils/ansible'; import { findProvidedModule, + getOrigRange, isBlockParam, isPlayParam, isRoleParam, isTaskParam, + parseAllDocuments, } from '../utils/yaml'; export const tokenTypes = [ @@ -233,9 +234,10 @@ function markNode( builder: SemanticTokensBuilder, document: TextDocument ) { - if (node.range) { - const startPosition = document.positionAt(node.range[0]); - const length = node.range[1] - node.range[0]; + const range = getOrigRange(node); + if (range) { + const startPosition = document.positionAt(range[0]); + const length = range[1] - range[0]; builder.push( startPosition.line, startPosition.character, diff --git a/src/providers/validationProvider.ts b/src/providers/validationProvider.ts index eeecb362..2f44a2f1 100644 --- a/src/providers/validationProvider.ts +++ b/src/providers/validationProvider.ts @@ -8,9 +8,9 @@ import { Range, } from 'vscode-languageserver'; import { TextDocument } from 'vscode-languageserver-textdocument'; -import { parseAllDocuments } from 'yaml'; import { ValidationManager } from '../services/validationManager'; import { WorkspaceFolderContext } from '../services/workspaceManager'; +import { parseAllDocuments } from '../utils/yaml'; /** * Validates the given document. @@ -63,8 +63,14 @@ function getYamlValidation(textDocument: TextDocument): Diagnostic[] { const errorRange = error.range || error.source?.range; let range; if (errorRange) { - const start = textDocument.positionAt(errorRange.start); - const end = textDocument.positionAt(errorRange.end); + const start = textDocument.positionAt( + errorRange.origStart !== undefined + ? errorRange.origStart + : errorRange.start + ); + const end = textDocument.positionAt( + errorRange.origEnd !== undefined ? errorRange.origEnd : errorRange.end + ); range = Range.create(start, end); let severity; diff --git a/src/services/ansibleConfig.ts b/src/services/ansibleConfig.ts index 495c4f4f..b188bee2 100644 --- a/src/services/ansibleConfig.ts +++ b/src/services/ansibleConfig.ts @@ -35,7 +35,7 @@ export class AnsibleConfig { const ansibleConfigResult = child_process.execSync(ansibleConfigCommand, { encoding: 'utf-8', - cwd: new URL(this.context.workspaceFolder.uri).pathname, + cwd: decodeURI(new URL(this.context.workspaceFolder.uri).pathname), env: ansibleConfigEnv, }); diff --git a/src/services/ansibleLint.ts b/src/services/ansibleLint.ts index af489c6f..0f7308c9 100644 --- a/src/services/ansibleLint.ts +++ b/src/services/ansibleLint.ts @@ -1,6 +1,7 @@ import * as child_process from 'child_process'; import { ExecException } from 'child_process'; import { promises as fs } from 'fs'; +import * as path from 'path'; import { URL } from 'url'; import { promisify } from 'util'; import { @@ -50,25 +51,47 @@ export class AnsibleLint { public async doValidate( textDocument: TextDocument ): Promise> { - const docPath = new URL(textDocument.uri).pathname; let diagnostics: Map = new Map(); - let progressTracker; - if (this.useProgressTracker) { - progressTracker = await this.connection.window.createWorkDoneProgress(); - } - const ansibleLintConfigPromise = this.getAnsibleLintConfig( - textDocument.uri + const workingDirectory = decodeURI( + new URL(this.context.workspaceFolder.uri).pathname ); - const workingDirectory = new URL(this.context.workspaceFolder.uri).pathname; + const settings = await this.context.documentSettings.get(textDocument.uri); - try { - const settings = await this.context.documentSettings.get( - textDocument.uri + if (settings.ansibleLint.enabled) { + let linterArguments = settings.ansibleLint.arguments; + + // Determine linter config file + let ansibleLintConfigPath = linterArguments.match( + /(?:^|\s)-c\s*(?[\s'"])(?.+?)(?:\k|$)/ + )?.groups?.conf; + if (!ansibleLintConfigPath) { + // Config file not provided in arguments -> search for one mimicking the + // way ansible-lint looks for it, going up the directory structure + const ansibleLintConfigFile = await this.findAnsibleLintConfigFile( + textDocument.uri + ); + if (ansibleLintConfigFile) { + ansibleLintConfigPath = decodeURI( + new URL(ansibleLintConfigFile).pathname + ); + linterArguments = `${linterArguments} -c "${ansibleLintConfigPath}"`; + } + } + linterArguments = `${linterArguments} --offline --nocolor -f codeclimate`; + + const docPath = decodeURI(new URL(textDocument.uri).pathname); + let progressTracker; + if (this.useProgressTracker) { + progressTracker = await this.connection.window.createWorkDoneProgress(); + } + const ansibleLintConfigPromise = this.getAnsibleLintConfig( + workingDirectory, + ansibleLintConfigPath ); - if (settings.ansibleLint.enabled) { + try { if (progressTracker) { progressTracker.begin( 'ansible-lint', @@ -79,7 +102,7 @@ export class AnsibleLint { const [command, env] = withInterpreter( settings.ansibleLint.path, - `${settings.ansibleLint.arguments} --offline --nocolor -f codeclimate ${docPath}`, + `${linterArguments} --offline --nocolor -f codeclimate "${docPath}"`, settings.python.interpreterPath, settings.python.activationScript ); @@ -98,36 +121,36 @@ export class AnsibleLint { if (result.stderr) { this.connection.console.info(`[ansible-lint] ${result.stderr}`); } - } - } catch (error) { - if (error instanceof Error) { - const execError = error as ExecException & { - // according to the docs, these are always available - stdout: string; - stderr: string; - }; - if (execError.code === 2) { - diagnostics = this.processReport( - execError.stdout, - await ansibleLintConfigPromise, - workingDirectory - ); - } else { - this.connection.window.showErrorMessage(execError.message); - } + } catch (error) { + if (error instanceof Error) { + const execError = error as ExecException & { + // according to the docs, these are always available + stdout: string; + stderr: string; + }; + if (execError.code === 2) { + diagnostics = this.processReport( + execError.stdout, + await ansibleLintConfigPromise, + workingDirectory + ); + } else { + this.connection.window.showErrorMessage(execError.message); + } - if (execError.stderr) { - this.connection.console.info(`[ansible-lint] ${execError.stderr}`); + if (execError.stderr) { + this.connection.console.info(`[ansible-lint] ${execError.stderr}`); + } + } else { + this.connection.console.error( + `Exception in AnsibleLint service: ${JSON.stringify(error)}` + ); } - } else { - this.connection.console.error( - `Exception in AnsibleLint service: ${JSON.stringify(error)}` - ); } - } - if (progressTracker) { - progressTracker.done(); + if (progressTracker) { + progressTracker.done(); + } } return diagnostics; } @@ -192,8 +215,8 @@ export class AnsibleLint { } } } - - const locationUri = `file://${workingDirectory}/${item.location.path}`; + const path = `${workingDirectory}/${item.location.path}`; + const locationUri = `file://${encodeURI(path)}`; let fileDiagnostics = diagnostics.get(locationUri); if (!fileDiagnostics) { @@ -242,14 +265,15 @@ export class AnsibleLint { } private async getAnsibleLintConfig( - uri: string + workingDirectory: string, + configPath: string | undefined ): Promise { - const configPath = await this.getAnsibleLintConfigPath(uri); if (configPath) { - let config = this.configCache.get(configPath); + const absConfigPath = path.resolve(workingDirectory, configPath); + let config = this.configCache.get(absConfigPath); if (!config) { - config = await this.readAnsibleLintConfig(configPath); - this.configCache.set(configPath, config); + config = await this.readAnsibleLintConfig(absConfigPath); + this.configCache.set(absConfigPath, config); } return config; } @@ -262,7 +286,7 @@ export class AnsibleLint { warnList: new Set(), }; try { - const configContents = await fs.readFile(new URL(configPath), { + const configContents = await fs.readFile(configPath, { encoding: 'utf8', }); parseAllDocuments(configContents).forEach((configDoc) => { @@ -284,7 +308,7 @@ export class AnsibleLint { return config; } - private async getAnsibleLintConfigPath( + private async findAnsibleLintConfigFile( uri: string ): Promise { // find configuration path diff --git a/src/utils/yaml.ts b/src/utils/yaml.ts index 14dcd021..a68c7662 100644 --- a/src/utils/yaml.ts +++ b/src/utils/yaml.ts @@ -1,6 +1,6 @@ import * as _ from 'lodash'; import { Position, TextDocument } from 'vscode-languageserver-textdocument'; -import { Document } from 'yaml'; +import { Document, Options, parseCST } from 'yaml'; import { Node, Pair, Scalar, YAMLMap, YAMLSeq } from 'yaml/types'; import { IModuleMetadata } from '../interfaces/module'; import { DocsLibrary } from '../services/docsLibrary'; @@ -131,7 +131,7 @@ export class AncestryBuilder { export function getPathAt( document: TextDocument, position: Position, - docs: Document.Parsed[], + docs: Document[], inclusive = false ): Node[] | null { const offset = document.offsetAt(position); @@ -147,10 +147,11 @@ export function contains( offset: number, inclusive: boolean ): boolean { + const range = getOrigRange(node); return !!( - node?.range && - node.range[0] <= offset && - (node.range[1] > offset || (inclusive && node.range[1] >= offset)) + range && + range[0] <= offset && + (range[1] > offset || (inclusive && range[1] >= offset)) ); } @@ -180,8 +181,8 @@ export function getPathAtOffset( } pair = _.find(currentNode.items, (p) => { const inBetweenNode = new Node(); - const start = (p.key as Node)?.range?.[1]; - const end = (p.value as Node)?.range?.[0]; + const start = getOrigRange(p.key as Node)?.[1]; + const end = getOrigRange(p.value as Node)?.[0]; if (start && end) { inBetweenNode.range = [start, end - 1]; return contains(inBetweenNode, offset, inclusive); @@ -390,3 +391,33 @@ export function getYamlMapKeys(mapNode: YAMLMap): Array { } }); } + +export function getOrigRange( + node: Node | null | undefined +): [number, number] | null | undefined { + if (node?.cstNode?.range) { + const range = node.cstNode.range; + return [ + range.origStart !== undefined ? range.origStart : range.start, + range.origEnd !== undefined ? range.origEnd : range.end, + ]; + } else { + return node?.range; + } +} + +/** Parsing with the YAML library tailored to the needs of this extension */ +export function parseAllDocuments(str: string, options?: Options): Document[] { + const cst = parseCST(str); + cst.setOrigRanges(); + + const parsedDocuments: Document[] = []; + for (const cstDoc of cst) { + const parsedDocument = new Document( + Object.assign({ keepCstNodes: true }, options) + ); + parsedDocument.parse(cstDoc); + parsedDocuments.push(parsedDocument); + } + return parsedDocuments; +}