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.

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

Use broader template-lint range #418

Merged
merged 13 commits into from
Nov 27, 2024
26 changes: 2 additions & 24 deletions src/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,8 @@ import {
getRegistryForRoots,
existsInRegistry,
} from './utils/registry-api';
import { ProjectProviders, collectProjectProviders, AddonMeta, DependencyMeta, emptyProjectProviders } from './utils/addon-api';
import {
findTestsForProject,
findAddonItemsForProject,
findAppItemsForProject,
isRootStartingWithFilePath,
getDepIfExists,
cached,
PackageInfo,
} from './utils/layout-helpers';
import { ProjectProviders, collectProjectProviders, AddonMeta, emptyProjectProviders } from './utils/addon-api';
import { findTestsForProject, findAddonItemsForProject, findAppItemsForProject, isRootStartingWithFilePath, cached, PackageInfo } from './utils/layout-helpers';
import { BaseProject } from './base-project';
import Server from './server';
import { Diagnostic, FileChangeType } from 'vscode-languageserver/node';
Expand Down Expand Up @@ -49,7 +41,6 @@ export class Project extends BaseProject {
providers!: ProjectProviders;
builtinProviders!: ProjectProviders;
addonsMeta: AddonMeta[] = [];
dependenciesMeta: DependencyMeta[] = [];
executors: Executors = {};
watchers: Watcher[] = [];
destructors: Destructor[] = [];
Expand Down Expand Up @@ -175,19 +166,6 @@ export class Project extends BaseProject {
this.addons = addons;
this.addonsMeta = [];
this._packageJSON = pkg;
// for now, let's collect only interesting deps
const interestingDeps = ['ember-cli', 'ember-source', 'ember-template-lint', 'typescript', '@embroider/core'];

interestingDeps.forEach((dep) => {
const version = getDepIfExists(pkg, dep);

if (version !== null) {
this.dependenciesMeta.push({
name: dep,
version,
});
}
});
}
async unload() {
this.initIssues = [];
Expand Down
49 changes: 36 additions & 13 deletions src/template-linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import Server from './server';
import { Project } from './project';
import { getRequireSupport } from './utils/layout-helpers';
import { getFileRanges, RangeWalker } from './utils/glimmer-script';
import * as semver from 'semver';
import { type SemVer } from 'semver';

type FindUp = (name: string, opts: { cwd: string; type: string }) => Promise<string | undefined>;
type LinterVerifyArgs = { source: string; moduleId: string; filePath: string };
Expand Down Expand Up @@ -88,7 +90,7 @@ export default class TemplateLinter {
return this.server.projectRoots.projectForUri(textDocument.uri);
}

private sourcesForDocument(textDocument: TextDocument, templateLintVersion: string): string[] {
private sourcesForDocument(textDocument: TextDocument, templateLintVersion: SemVer | null): string[] {
const ext = getExtension(textDocument);

if (ext !== null && !extensionsToLint.includes(ext)) {
Expand All @@ -98,8 +100,11 @@ export default class TemplateLinter {
const documentContent = textDocument.getText();

// we assume that ember-template-lint v5 could handle js/ts/gts/gjs files
if (!templateLintVersion) {
return [documentContent];
}

if (templateLintVersion === '5') {
if (semver.gte(templateLintVersion, '5.0.0')) {
return [documentContent];
}
Comment on lines +107 to 109
Copy link

@arthur5005 arthur5005 Nov 25, 2024

Choose a reason for hiding this comment

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

This doesn't work sadly.
Something like the following is technically correct:

semver.gte(semver.minVersion(templateLintVersion)?.version ?? '0.0.0', '5.0.0')


Expand Down Expand Up @@ -139,28 +144,46 @@ export default class TemplateLinter {
});
}
}
getSourcesForDocument(textDocument: TextDocument, project: Project): string[] {
const linterMeta = project.dependencyMap.get('ember-template-lint');
Copy link
Contributor

Choose a reason for hiding this comment

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

likely we need to add integration test for it, or test locally :)


if (!linterMeta) {
return [];
}

let sources = [];

try {
/**
* Semver parsing can throw errors, if the version is invalid,
* we want behave as if there was no version specified.
*
* (same as when errors are thrown from sourcesForDocument)
*/
const version = linterMeta?.package.version;
const linterVersion = version ? semver.parse(version) : null;

Choose a reason for hiding this comment

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

semver.minVersion(version)?.version ?? '0.0.0'

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated logic on how we resolve template-lint version, now it should be taken from deps, need to test it...


sources = this.sourcesForDocument(textDocument, linterVersion);
} catch (e) {
return [];
}

return sources;
}
async lint(textDocument: TextDocument): Promise<Diagnostic[] | undefined> {
if (this._isEnabled === false) {
return;
return [];
}

const cwd = process.cwd();

const project = this.getProjectForDocument(textDocument);

if (!project) {
return;
}

const linterMeta = project.dependenciesMeta.find((dep) => dep.name === 'ember-template-lint');
const linterVersion = linterMeta?.version.split('.')[0] || 'unknown';

let sources = [];

try {
sources = this.sourcesForDocument(textDocument, linterVersion);
} catch (e) {
return;
}
const sources = this.getSourcesForDocument(textDocument, project);

if (!sources.length) {
return;
Expand Down
1 change: 0 additions & 1 deletion src/utils/addon-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ export async function collectProjectProviders(root: string, addons: string[], de
}

export type AddonMeta = { root: string; name: string; version: null | 1 | 2 };
export type DependencyMeta = { name: string; version: string };

export function emptyProjectProviders(providers?: Partial<ProjectProviders>): ProjectProviders {
return {
Expand Down
6 changes: 2 additions & 4 deletions src/utils/layout-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as memoize from 'memoizee';
import * as path from 'path';
import { CompletionItem, CompletionItemKind } from 'vscode-languageserver/node';
import { addToRegistry, normalizeMatchNaming } from './registry-api';
import { clean, coerce, valid } from 'semver';
import { BaseProject } from '../base-project';
import { fsProvider } from '../fs-provider';
import walkAsync from './walk-async';
Expand Down Expand Up @@ -42,6 +41,7 @@ type StringConfig = Record<string, string>;
export interface PackageInfo {
keywords?: string[];
name?: string;
version?: string;
'ember-language-server'?: UnknownConfig;
peerDependencies?: StringConfig;
devDependencies?: StringConfig;
Expand Down Expand Up @@ -297,9 +297,7 @@ export function getDepIfExists(pack: PackageInfo, depName: string): string | nul

const version: string = pack?.dependencies?.[depName] ?? pack?.devDependencies?.[depName] ?? pack?.peerDependencies?.[depName] ?? '';

const cleanVersion = clean(version);

return valid(coerce(cleanVersion));
return version;
}

export async function isGlimmerXProject(root: string) {
Expand Down
122 changes: 122 additions & 0 deletions test/template-linter-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import TemplateLinter from '../src/template-linter';
import { type Project, type Server } from '../src';
import { type TextDocument } from 'vscode-languageserver-textdocument';

function getLinterInstance(depName?: string, depVersion?: string): [TemplateLinter, Project] {
const linter = new TemplateLinter({
projectRoots: {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
projectForUri(_url: string) {
return {
dependencyMap: new Map(depName ? [[depName, { package: { name: depName, version: depVersion } }]] : []),
} as Project;
},
},
options: {
type: 'node',
},
} as Server);

return [linter, linter['server'].projectRoots.projectForUri('') as Project];
}

describe('template-linter', function () {
describe('sourcesForDocument', function () {
it('supports empty template-lint version', function () {
const [linter, project] = getLinterInstance();

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([]);
});
it('supports incorrect template-lint version [foo-bar]', function () {
const [linter, project] = getLinterInstance('ember-template-lint', 'foo-bar');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([doc.getText()]);
});
it('supports incorrect template-lint version [*]', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '*');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([doc.getText()]);
});
it('process gjs for template-lint v2 with', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '2.0.0');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([' 1']);
});
it('process gjs for template-lint v3 with', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '3.3.1');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([' 1']);
});
it('process gjs for template-lint v4 with', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '4.3.1');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([' 1']);
});
it('skip gjs processing for template-lint v5', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '5.0.0');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([doc.getText()]);
});
it('skip gjs processing for template-lint v6', function () {
const [linter, project] = getLinterInstance('ember-template-lint', '6.0.0');

const doc: TextDocument = {
uri: 'test.gjs',
getText() {
return 'let a = 12;<template>1</template>';
},
} as TextDocument;

expect(linter.getSourcesForDocument(doc, project)).toEqual([doc.getText()]);
});
});
});
Loading