-
Notifications
You must be signed in to change notification settings - Fork 39
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
base: master
Are you sure you want to change the base?
Changes from 11 commits
035923e
514ae2f
1859807
2dd183c
827faef
10a09db
e2804b1
5e964ca
372197b
95e33c8
8b30ec5
e44d324
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 }; | ||
|
@@ -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)) { | ||
|
@@ -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]; | ||
} | ||
|
||
|
@@ -151,12 +156,24 @@ export default class TemplateLinter { | |
return; | ||
} | ||
|
||
const linterMeta = project.dependenciesMeta.find((dep) => dep.name === 'ember-template-lint'); | ||
const linterVersion = linterMeta?.version.split('.')[0] || 'unknown'; | ||
const linterMeta = project.dependencyMap.get('ember-template-lint'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import * as semver from 'semver'; | ||
|
||
import TemplateLinter from '../src/template-linter'; | ||
import { type Server } from '../src'; | ||
import { TextDocument } from 'vscode-languageserver-textdocument'; | ||
|
||
describe('template-linter', function () { | ||
describe('sourcesForDocument', function () { | ||
const linter = new TemplateLinter({ | ||
options: { | ||
type: 'node', | ||
}, | ||
} as Server); | ||
|
||
it('supports empty template-lint version', function () { | ||
const doc: TextDocument = { | ||
uri: 'test.gjs', | ||
getText() { | ||
return 'let a = 12;<template>1</template>'; | ||
}, | ||
} as TextDocument; | ||
|
||
expect(linter['sourcesForDocument'](doc, null)).toEqual(['let a = 12;<template>1</template>']); | ||
}); | ||
it('process gjs for template-lint v4 with ~', function () { | ||
const doc: TextDocument = { | ||
uri: 'test.gjs', | ||
getText() { | ||
return 'let a = 12;<template>1</template>'; | ||
}, | ||
} as TextDocument; | ||
|
||
console.log(semver.parse('~4.3.1')); // should be counted as 4.3.1 | ||
expect(linter['sourcesForDocument'](doc, semver.parse('~4.3.1'))).toEqual([' 1']); | ||
}); | ||
it('process gjs for template-lint v4 with ^', function () { | ||
const doc: TextDocument = { | ||
uri: 'test.gjs', | ||
getText() { | ||
return 'let a = 12;<template>1</template>'; | ||
}, | ||
} as TextDocument; | ||
|
||
console.log(semver.parse('^4.3.1')); // should be counted as 4.3.1 | ||
expect(linter['sourcesForDocument'](doc, semver.parse('^4.3.1'))).toEqual([' 1']); | ||
}); | ||
it('process gjs for template-lint v4 with strict dependency', function () { | ||
const doc: TextDocument = { | ||
uri: 'test.gjs', | ||
getText() { | ||
return 'let a = 12;<template>1</template>'; | ||
}, | ||
} as TextDocument; | ||
|
||
console.log(semver.parse('4.3.1')); // should be counted as 4.3.1 | ||
expect(linter['sourcesForDocument'](doc, semver.parse('4.3.1'))).toEqual([' 1']); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
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: