Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

Fallback to syntax-check if ansible-lint is not installed or disabled #5

Merged
merged 44 commits into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
0ef7af6
Add support for ansible syntax-check
priyamsahoo Aug 23, 2021
08c077a
Add utility function for library checking
priyamsahoo Aug 23, 2021
5a72cc6
Add logic to fallback to ansible syntax-check if ansible-lint is not …
priyamsahoo Aug 23, 2021
f7eb904
Resolve bug for overrunning the syntax-check service
priyamsahoo Aug 25, 2021
239c3c6
Remove copy-paste leftovers
priyamsahoo Aug 25, 2021
e95be14
Use ansible path to run syntax-check rather than having to create a n…
priyamsahoo Aug 25, 2021
420a0ec
Rename ansibleSyntaxCheck service to ansibleSyntaxChecker
priyamsahoo Aug 26, 2021
63a6419
Remove unused imports
priyamsahoo Aug 31, 2021
dc25ff7
add new package-lock.json
priyamsahoo Aug 31, 2021
4590c0b
Add support for ansible syntax-check
priyamsahoo Aug 23, 2021
d0fcdac
Remove copy-paste leftovers
priyamsahoo Aug 25, 2021
3ca5cae
Use ansible path to run syntax-check rather than having to create a n…
priyamsahoo Aug 25, 2021
dced3bc
Rename ansibleSyntaxCheck service to ansibleSyntaxChecker
priyamsahoo Aug 26, 2021
d9bf372
Remove unused imports
priyamsahoo Aug 31, 2021
c159795
Add fallback logic to the right place
priyamsahoo Aug 31, 2021
f9cebb6
Add support for fallback to ansible syntax-check when the ansible-lin…
priyamsahoo Aug 31, 2021
a89e808
New implementation of libraryChecker as getExecutablePath
priyamsahoo Aug 31, 2021
4a23964
Remaned AnsibleSyntaxChecker class to AnsiblePlaybook class
priyamsahoo Aug 31, 2021
bb18f33
Embed `private` attributes into the constructor declaration
priyamsahoo Sep 9, 2021
21dd1e4
Convert else block with guard expression @ doValidate for ansibleLint
priyamsahoo Sep 9, 2021
4367320
Make invoking progressTracker unconditional
priyamsahoo Sep 9, 2021
f0de9f5
Move await to the getExecutablePath method invocation
priyamsahoo Sep 9, 2021
1d033e6
Remove redundant child_process imports
priyamsahoo Sep 9, 2021
ad21769
Reformat AnsiblePlaybook constructor
priyamsahoo Sep 9, 2021
d5e46f1
Refactor processReport method @ AnsiblePlaybook
priyamsahoo Sep 9, 2021
8a9f59d
Remove unnecessary statements from try-catch block
priyamsahoo Sep 13, 2021
0d38292
Use config variable in getExecutablePath in place of hardcoded string
priyamsahoo Sep 15, 2021
064d23a
Add new package-lock.json
priyamsahoo Sep 15, 2021
4613993
Merge branch 'main' into syntax-check
priyamsahoo Sep 30, 2021
c6ab5c6
sync package-lock.json
priyamsahoo Sep 30, 2021
09ac750
revert the gramatical change to have it fixed in a separate PR.
priyamsahoo Sep 30, 2021
78212ce
separate the imports from exec initialization
priyamsahoo Sep 30, 2021
6e82a13
add jsdoc for the AnsiblePlaybook constructor
priyamsahoo Sep 30, 2021
035bb33
use URI from vscode-uri instead of URL
priyamsahoo Sep 30, 2021
ff84617
change variable name from ansibleSyntaxCheck to ansiblePlaybook
priyamsahoo Sep 30, 2021
f7ffbd8
Merge branch 'main' into syntax-check
priyamsahoo Oct 5, 2021
a272ae6
chore: auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 5, 2021
e01b8db
Change variable name to something more sensible
priyamsahoo Oct 5, 2021
589f0c9
Merge branch 'main' into syntax-check
priyamsahoo Oct 5, 2021
4f32dd8
Merge branch 'syntax-check' of github.com:priyamsahoo/ansible-languag…
priyamsahoo Oct 5, 2021
e5dc2f3
remove duplicate import statements
priyamsahoo Oct 6, 2021
6afb113
add connection parameter to doValidate function in validation provider
priyamsahoo Oct 7, 2021
c41d8d8
move syntax-check notification to from ansible lint service to valida…
priyamsahoo Oct 7, 2021
58b1971
Merge branch 'main' into syntax-check
ssbarnea Oct 8, 2021
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
19 changes: 16 additions & 3 deletions src/ansibleLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,13 @@ export class AnsibleLanguageService {
const context = this.workspaceManager.getContext(e.document.uri);
if (context) {
// perform full validation
await doValidate(e.document, this.validationManager, false, context);
await doValidate(
e.document,
this.validationManager,
false,
context,
this.connection
);
}
} catch (error) {
this.handleError(error, 'onDidOpen');
Expand Down Expand Up @@ -177,7 +183,13 @@ export class AnsibleLanguageService {
const context = this.workspaceManager.getContext(e.document.uri);
if (context) {
// perform full validation
await doValidate(e.document, this.validationManager, false, context);
await doValidate(
e.document,
this.validationManager,
false,
context,
this.connection
);
}
} catch (error) {
this.handleError(error, 'onDidSave');
Expand All @@ -201,7 +213,8 @@ export class AnsibleLanguageService {
e.document,
this.validationManager,
true,
this.workspaceManager.getContext(e.document.uri)
this.workspaceManager.getContext(e.document.uri),
this.connection
);
} catch (error) {
this.handleError(error, 'onDidChangeContent');
Expand Down
35 changes: 32 additions & 3 deletions src/providers/validationProvider.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import IntervalTree from '@flatten-js/interval-tree';
import * as _ from 'lodash';
import {
Connection,
Diagnostic,
DiagnosticRelatedInformation,
DiagnosticSeverity,
Expand All @@ -11,6 +12,7 @@ import { TextDocument } from 'vscode-languageserver-textdocument';
import { ValidationManager } from '../services/validationManager';
import { WorkspaceFolderContext } from '../services/workspaceManager';
import { parseAllDocuments } from '../utils/yaml';
import { getExecutablePath } from '../utils/misc';
priyamsahoo marked this conversation as resolved.
Show resolved Hide resolved

/**
* Validates the given document.
Expand All @@ -23,7 +25,8 @@ export async function doValidate(
textDocument: TextDocument,
validationManager: ValidationManager,
quick = true,
context?: WorkspaceFolderContext
context?: WorkspaceFolderContext,
connection?: Connection
): Promise<Map<string, Diagnostic[]>> {
let diagnosticsByFile;
if (quick || !context) {
Expand All @@ -32,8 +35,34 @@ export async function doValidate(
validationManager.getValidationFromCache(textDocument.uri) ||
new Map<string, Diagnostic[]>();
} else {
// full validation with ansible-lint
diagnosticsByFile = await context.ansibleLint.doValidate(textDocument);
// full validation with ansible-lint or ansible syntax-check (if ansible-lint is not installed or disabled)

const settings = await context.documentSettings.get(textDocument.uri);

const lintAvailability = await getExecutablePath(settings.ansibleLint.path);
console.debug('Path for lint: ', lintAvailability);

if (lintAvailability) {
console.debug('Validating using ansible-lint');
diagnosticsByFile = await context.ansibleLint.doValidate(textDocument);
}

if (!diagnosticsByFile || !lintAvailability || diagnosticsByFile === -1) {
// Notifying the user about the failed ansible-lint command and falling back to ansible syntax-check in this scenario
if (diagnosticsByFile === -1) {
console.debug(
'Ansible-lint command execution failed. Falling back to ansible syntax-check'
);
connection.window.showInformationMessage(
'Falling back to ansible syntax-check.'
);
}
console.debug('Validating using ansible syntax-check');
diagnosticsByFile = await context.ansiblePlaybook.doValidate(
textDocument
);
}

if (!diagnosticsByFile.has(textDocument.uri)) {
// In case there are no diagnostics for the file that triggered the
// validation, set an empty array in order to clear the validation.
Expand Down
10 changes: 8 additions & 2 deletions src/services/ansibleLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,19 @@ export class AnsibleLint {
*/
public async doValidate(
textDocument: TextDocument
): Promise<Map<string, Diagnostic[]>> {
): Promise<Map<string, Diagnostic[]> | -1> {
let diagnostics: Map<string, Diagnostic[]> = new Map();

const workingDirectory = URI.parse(this.context.workspaceFolder.uri).path;

const settings = await this.context.documentSettings.get(textDocument.uri);

if (settings.ansibleLint.enabled) {
if (!settings.ansibleLint.enabled) {
console.debug(
'Ansible-lint is disabled. Falling back to ansible syntax-check'
);
return;
} else {
let linterArguments = settings.ansibleLint.arguments;

// Determine linter config file
Expand Down Expand Up @@ -132,6 +137,7 @@ export class AnsibleLint {
);
} else {
this.connection.window.showErrorMessage(execError.message);
return -1;
ganeshrn marked this conversation as resolved.
Show resolved Hide resolved
}

if (execError.stderr) {
Expand Down
160 changes: 160 additions & 0 deletions src/services/ansiblePlaybook.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
import * as child_process from 'child_process';
import { URI } from 'vscode-uri';
import { promisify } from 'util';
import {
Connection,
Diagnostic,
DiagnosticSeverity,
Position,
Range,
} from 'vscode-languageserver';
import { TextDocument } from 'vscode-languageserver-textdocument';
import { withInterpreter } from '../utils/misc';
import { WorkspaceFolderContext } from './workspaceManager';

const exec = promisify(child_process.exec);
webknjaz marked this conversation as resolved.
Show resolved Hide resolved

/**
* Acts as an interface to ansible-playbook command.
*/
export class AnsiblePlaybook {
priyamsahoo marked this conversation as resolved.
Show resolved Hide resolved
private useProgressTracker = false;

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is missing some text description.

*
* @param connection establishes connection with the client
* @param context provides workspace context of the client
*/

constructor(
private connection: Connection,
private context: WorkspaceFolderContext
) {
this.useProgressTracker =
!!context.clientCapabilities.window?.workDoneProgress;
}

/**
* Acts as an interface to ansible-playbook <file> --syntax-check command and a cache of its output.
* ansible syntax-check may provide diagnostics for more than just the file for which
* it was triggered, and this is reflected in the implementation.
*
* Perform ansible syntax-check for the given document.
*/
public async doValidate(
textDocument: TextDocument
): Promise<Map<string, Diagnostic[]>> {
const docPath = URI.parse(textDocument.uri).path;
let diagnostics: Map<string, Diagnostic[]> = new Map();
const progressTracker = this.useProgressTracker
? await this.connection.window.createWorkDoneProgress()
: {
begin: () => {}, // eslint-disable-line @typescript-eslint/no-empty-function
done: () => {}, // eslint-disable-line @typescript-eslint/no-empty-function
};

const workingDirectory = URI.parse(this.context.workspaceFolder.uri).path;

const settings = await this.context.documentSettings.get(textDocument.uri);

progressTracker.begin(
'ansible syntax-check',
undefined,
'Processing files...'
);

const [command, env] = withInterpreter(
`${settings.ansible.path}-playbook`,
`${docPath} --syntax-check`,
settings.python.interpreterPath,
settings.python.activationScript
);

try {
await exec(command, {
encoding: 'utf-8',
cwd: workingDirectory,
env: env,
});
} catch (error) {
if (error instanceof Error) {
const execError = error as child_process.ExecException & {
// according to the docs, these are always available
stdout: string;
stderr: string;
};

// This is the regex to extract the filename, line and column number from the strerr produced by syntax-check command
const ansibleSyntaxCheckRegex =
/The error appears to be in '(?<filename>.*)': line (?<line>\d+), column (?<column>\d+)/;

const filteredErrorMessage = ansibleSyntaxCheckRegex.exec(execError.stderr);

diagnostics = filteredErrorMessage
? this.processReport(
execError.message,
filteredErrorMessage.groups.filename,
parseInt(filteredErrorMessage.groups.line),
parseInt(filteredErrorMessage.groups.column)
)
: this.processReport(execError.message, docPath, 1, 1);

if (execError.stderr) {
this.connection.console.info(
`[ansible syntax-check] ${execError.stderr}`
);
}
} else {
this.connection.console.error(
`Exception in AnsibleSyntaxCheck service: ${JSON.stringify(error)}`
);
}
}

progressTracker.done();
return diagnostics;
}

private processReport(
result: string,
fileName: string,
line: number,
column: number
): Map<string, Diagnostic[]> {
const diagnostics: Map<string, Diagnostic[]> = new Map();
if (!result) {
tomaciazek marked this conversation as resolved.
Show resolved Hide resolved
this.connection.console.warn(
'Standard output from ansible syntax-check is suspiciously empty.'
);
return diagnostics;
}
const start: Position = {
line: line - 1,
character: column - 1,
};
const end: Position = {
line: line - 1,
character: Number.MAX_SAFE_INTEGER,
};
const range: Range = {
start,
end,
};

const severity: DiagnosticSeverity = DiagnosticSeverity.Error;

const locationUri = `file://${fileName}`;

const fileDiagnostics = diagnostics.get(locationUri) || [];

fileDiagnostics.push({
message: result,
range,
severity,
source: 'Ansible',
});

diagnostics.set(locationUri, fileDiagnostics);
return diagnostics;
}
}
9 changes: 9 additions & 0 deletions src/services/workspaceManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'vscode-languageserver';
import { AnsibleConfig } from './ansibleConfig';
import { AnsibleLint } from './ansibleLint';
import { AnsiblePlaybook } from './ansiblePlaybook';
import { DocsLibrary } from './docsLibrary';
import { ExecutionEnvironment } from './executionEnvironment';
import { MetadataLibrary } from './metadataLibrary';
Expand Down Expand Up @@ -113,6 +114,7 @@ export class WorkspaceFolderContext {
private _docsLibrary: Thenable<DocsLibrary> | undefined;
private _ansibleConfig: Thenable<AnsibleConfig> | undefined;
private _ansibleLint: AnsibleLint | undefined;
private _ansiblePlaybook: AnsiblePlaybook | undefined;

constructor(
connection: Connection,
Expand Down Expand Up @@ -180,6 +182,13 @@ export class WorkspaceFolderContext {
return this._ansibleLint;
}

public get ansiblePlaybook(): AnsiblePlaybook {
if (!this._ansiblePlaybook) {
this._ansiblePlaybook = new AnsiblePlaybook(this.connection, this);
}
return this._ansiblePlaybook;
}

public get executionEnvironment(): Thenable<ExecutionEnvironment> {
if (!this._executionEnvironment) {
const executionEnvironment = new ExecutionEnvironment(
Expand Down
20 changes: 20 additions & 0 deletions src/utils/misc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,23 @@ export function withInterpreter(
return [command, undefined];
}
}

/**
* A method to return the path to the provided executable
* @param executable String representing the name of the executable
* @returns Complete path of the executable (string) or undefined depending updon the presence of the executable
*/
export async function getExecutablePath(
executable: string
): Promise<string> | undefined {
const exec = promisify(child_process.exec);

try {
const executablePath = await exec(`which ${executable}`, {
encoding: 'utf-8',
});
return executablePath.stdout;
} catch (error) {
return;
}
}