Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: Warn for old Stylelint regardless of language ID #340

Merged
merged 1 commit into from
Nov 20, 2021
Merged

feat: Warn for old Stylelint regardless of language ID #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 67 additions & 27 deletions src/server/modules/__tests__/old-stylelint-warning.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,16 @@ const mockedGetWorkspaceFolder = getWorkspaceFolder as jest.MockedFunction<
>;
const mockedFindPackageRoot = findPackageRoot as jest.MockedFunction<typeof findPackageRoot>;

const mockContext = serverMocks.getContext();
const mockLogger = serverMocks.getLogger();
let mockContext: ReturnType<typeof serverMocks.getContext>;
let mockLogger: ReturnType<typeof serverMocks.getLogger>;

describe('OldStylelintWarningModule', () => {
beforeEach(() => {
jest.clearAllMocks();
mockedPath.__mockPlatform('posix');
mockContext = serverMocks.getContext();
mockLogger = serverMocks.getLogger();
mockContext.__options.validate = [];
jest.clearAllMocks();
});

test('should be constructable', () => {
Expand All @@ -42,8 +44,9 @@ describe('OldStylelintWarningModule', () => {
expect(mockContext.documents.onDidOpen).toHaveBeenCalledWith(expect.any(Function));
});

test('if document language ID is not in options, should not warn', async () => {
mockContext.__options.validate = ['baz'];
test('if document is not part of a workspace, should not warn', async () => {
mockedGetWorkspaceFolder.mockResolvedValue(undefined);
mockContext.__options.validate = ['bar'];

const module = new OldStylelintWarningModule({
context: mockContext.__typed(),
Expand All @@ -63,13 +66,13 @@ describe('OldStylelintWarningModule', () => {
expect(mockContext.resolveStylelint).not.toHaveBeenCalled();
expect(mockContext.connection.window.showWarningMessage).not.toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenLastCalledWith(
'Document should not be validated, ignoring',
{ uri: 'foo', language: 'bar' },
'Document not part of a workspace, ignoring',
{ uri: 'foo' },
);
});

test('if document is not part of a workspace, should not warn', async () => {
mockedGetWorkspaceFolder.mockResolvedValue(undefined);
test('if document has already been checked, should not warn', async () => {
mockedGetWorkspaceFolder.mockResolvedValue('/path');
mockContext.__options.validate = ['bar'];

const module = new OldStylelintWarningModule({
Expand All @@ -81,22 +84,27 @@ describe('OldStylelintWarningModule', () => {

module.onDidRegisterHandlers();

const document = TextDocument.create('foo', 'bar', 1, 'a {}');
const handler = mockContext.documents.onDidOpen.mock.calls[0][0];

await handler({
document: TextDocument.create('foo', 'bar', 1, 'a {}'),
});
await handler({ document });
await handler({ document });

expect(mockContext.resolveStylelint).not.toHaveBeenCalled();
expect(mockContext.resolveStylelint).toHaveBeenCalledTimes(1);
expect(mockContext.connection.window.showWarningMessage).not.toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenLastCalledWith(
'Document not part of a workspace, ignoring',
'Document has already been checked, ignoring',
{ uri: 'foo' },
);
});

test('if document has already been checked, should not warn', async () => {
test('if Stylelint package root cannot be determined, should not warn', async () => {
mockedGetWorkspaceFolder.mockResolvedValue('/path');
mockedFindPackageRoot.mockResolvedValue(undefined);
mockContext.resolveStylelint.mockResolvedValue({
stylelint: {} as unknown as stylelint.PublicApi,
resolvedPath: '/path/node_modules/stylelint',
});
mockContext.__options.validate = ['bar'];

const module = new OldStylelintWarningModule({
Expand All @@ -108,28 +116,30 @@ describe('OldStylelintWarningModule', () => {

module.onDidRegisterHandlers();

const document = TextDocument.create('foo', 'bar', 1, 'a {}');
const handler = mockContext.documents.onDidOpen.mock.calls[0][0];

await handler({ document });
await handler({ document });
await handler({
document: TextDocument.create('foo', 'bar', 1, 'a {}'),
});

expect(mockContext.resolveStylelint).toHaveBeenCalledTimes(1);
expect(mockContext.connection.window.showWarningMessage).not.toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenLastCalledWith(
'Document has already been checked, ignoring',
{ uri: 'foo' },
);
expect(mockLogger.debug).toHaveBeenLastCalledWith('Stylelint package root not found', {
uri: 'foo',
});
});

test('if Stylelint package root cannot be determined, should not warn', async () => {
test('if Stylelint package root is not in workspace, should not warn', async () => {
const error = new Error('foo');

mockedGetWorkspaceFolder.mockResolvedValue('/path');
mockedFindPackageRoot.mockResolvedValue(undefined);
mockedFindPackageRoot.mockResolvedValue('/usr/local/lib/node_modules/stylelint');
mockContext.resolveStylelint.mockResolvedValue({
stylelint: {} as unknown as stylelint.PublicApi,
resolvedPath: '/path/node_modules/stylelint',
});
mockContext.__options.validate = ['bar'];
mockedFS.readFile.mockRejectedValue(error);

const module = new OldStylelintWarningModule({
context: mockContext.__typed(),
Expand All @@ -148,9 +158,10 @@ describe('OldStylelintWarningModule', () => {

expect(mockContext.resolveStylelint).toHaveBeenCalledTimes(1);
expect(mockContext.connection.window.showWarningMessage).not.toHaveBeenCalled();
expect(mockLogger.debug).toHaveBeenLastCalledWith('Stylelint package root not found', {
uri: 'foo',
});
expect(mockLogger.debug).toHaveBeenLastCalledWith(
'Stylelint package root is not inside the workspace',
{ uri: 'foo' },
);
});

test('if Stylelint package manifest cannot be read, should not warn', async () => {
Expand Down Expand Up @@ -501,6 +512,35 @@ describe('OldStylelintWarningModule', () => {
expect(mockLogger.warn).toHaveBeenCalledWith('Failed to open migration guide');
});

test('if document language ID is not in options, should warn', async () => {
mockedGetWorkspaceFolder.mockResolvedValue('/path');
mockedFindPackageRoot.mockResolvedValue('/path/node_modules/stylelint');
mockContext.resolveStylelint.mockResolvedValue({
stylelint: {} as unknown as stylelint.PublicApi,
resolvedPath: '/path/node_modules/stylelint',
});
mockContext.__options.validate = ['baz'];

const module = new OldStylelintWarningModule({
context: mockContext.__typed(),
logger: mockLogger,
});

module.onInitialize({ capabilities: {} } as unknown as LSP.InitializeParams);

module.onDidRegisterHandlers();

const handler = mockContext.documents.onDidOpen.mock.calls[0][0];

await handler({
document: TextDocument.create('foo', 'bar', 1, 'a {}'),
});

expect(mockContext.resolveStylelint).toHaveBeenCalledTimes(1);
expect(mockContext.connection.window.showWarningMessage).toHaveBeenCalledTimes(1);
expect(mockContext.connection.window.showDocument).not.toHaveBeenCalled();
});

it('should be disposable', () => {
const module = new OldStylelintWarningModule({
context: mockContext.__typed(),
Expand Down
27 changes: 14 additions & 13 deletions src/server/modules/old-stylelint-warning.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import path from 'path';
import fs from 'fs/promises';
import semver from 'semver';
import pathIsInside from 'path-is-inside';
import type { TextDocument } from 'vscode-languageserver-textdocument';
import type LSP from 'vscode-languageserver-protocol';
import { getWorkspaceFolder } from '../../utils/documents';
Expand Down Expand Up @@ -54,7 +55,10 @@ export class OldStylelintWarningModule implements LanguageServerModule {
this.#openMigrationGuide = capabilities.window?.showDocument?.support ?? false;
}

async #getStylelintVersion(document: TextDocument): Promise<string | undefined> {
async #getStylelintVersion(
document: TextDocument,
workspaceFolder: string,
): Promise<string | undefined> {
const result = await this.#context.resolveStylelint(document);

if (!result) {
Expand All @@ -75,6 +79,14 @@ export class OldStylelintWarningModule implements LanguageServerModule {
return undefined;
}

if (!pathIsInside(packageDir, workspaceFolder)) {
this.#logger?.debug('Stylelint package root is not inside the workspace', {
uri: document.uri,
});

return undefined;
}

const manifestPath = path.join(packageDir, 'package.json');

try {
Expand All @@ -95,17 +107,6 @@ export class OldStylelintWarningModule implements LanguageServerModule {
}

async #check(document: TextDocument): Promise<string | undefined> {
const options = await this.#context.getOptions(document.uri);

if (!options.validate.includes(document.languageId)) {
this.#logger?.debug('Document should not be validated, ignoring', {
uri: document.uri,
language: document.languageId,
});

return undefined;
}

const workspaceFolder = await getWorkspaceFolder(this.#context.connection, document);

if (!workspaceFolder) {
Expand All @@ -126,7 +127,7 @@ export class OldStylelintWarningModule implements LanguageServerModule {

this.#checkedWorkspaces.add(workspaceFolder);

const stylelintVersion = await this.#getStylelintVersion(document);
const stylelintVersion = await this.#getStylelintVersion(document, workspaceFolder);

if (!stylelintVersion) {
return undefined;
Expand Down