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

Add TypeScript type-checking to all JavaScript files and publish utility types #723

Merged
merged 1 commit into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,13 @@ module.exports = {
'unicorn/filename-case': 'off',
},
},
{
parser: '@typescript-eslint/parser',
files: ['*.ts'],
extends: ['plugin:@typescript-eslint/recommended'],
rules: {
'node/no-unsupported-features/es-syntax': 'off',
},
},
],
};
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,6 @@ node_modules

# eslint
.eslintcache

# TypeScript output
dist/
23 changes: 11 additions & 12 deletions lib/rules/no-assert-ok-find.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';

const isMemberExpression = require('../utils/is-member-expression');
const isTestFile = require('../utils/is-test-file');
const { ReferenceTracker } = require('eslint-utils');

Expand Down Expand Up @@ -36,7 +35,17 @@ module.exports = {

for (const { node } of tracker.iterateGlobalReferences(traceMap)) {
const parentNode = node.parent;
if (isAssertOk(parentNode)) {
if (
// Check for assert.ok(...)
parentNode.type === 'CallExpression' &&
parentNode.callee.type === 'MemberExpression' &&
parentNode.callee.object.type === 'Identifier' &&
parentNode.callee.object.name === 'assert' &&
parentNode.callee.property.type === 'Identifier' &&
parentNode.callee.property.name === 'ok' &&
(parentNode.arguments.length === 1 ||
parentNode.arguments.length === 2)
) {
context.report({
node: parentNode,
messageId: 'error',
Expand Down Expand Up @@ -68,13 +77,3 @@ module.exports = {
};
},
};

function isAssertOk(node) {
return (
node.type === 'CallExpression' &&
isMemberExpression(node.callee) &&
node.callee.object.name === 'assert' &&
node.callee.property.name === 'ok' &&
(node.arguments.length === 1 || node.arguments.length === 2)
);
}
4 changes: 4 additions & 0 deletions lib/rules/no-handlebar-interpolation.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ module.exports = {
// Defaults to anything `*.js`
const defaultFilePattern = /\.js$/;
// Only run on files that match filePatterns.
/** @type {{filePatterns: RegExp[]}} */
const { filePatterns = [defaultFilePattern] } = context.options[0] || {};
const matchesFilename = filePatterns.some((pattern) => {
return pattern.test(context.getFilename());
Expand All @@ -46,6 +47,9 @@ module.exports = {
return {};
}

/**
* @param {import('estree').Literal} node
*/
function checkNode(node) {
const { value } = node;
if (typeof value === 'string' && tripleStashRegex.test(value)) {
Expand Down
4 changes: 3 additions & 1 deletion lib/rules/no-missing-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ module.exports = {
},
},
create(context) {
const matchingLocation = context.options[0].find((location) =>
/** @type {{filePath:string,testPaths:string[],hasTestSuffix?:boolean}[]} */
const config = context.options[0];
const matchingLocation = config.find((location) =>
context.getFilename().includes(location.filePath)
);

Expand Down
5 changes: 4 additions & 1 deletion lib/rules/no-restricted-files.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,15 @@ module.exports = {
create(context) {
return {
Program(node) {
const match = context.options.find((option) =>
/** @type {{message?:string,paths:string[]}[]} */
const config = context.options;
const match = config.find((option) =>
option.paths.some((path) => context.getFilename().match(path))
);
if (match) {
context.report({
node,
// @ts-ignore -- `messageId` is optional.
messageId: match.message ? undefined : 'defaultError',
message: match.message, // eslint-disable-line eslint-plugin/prefer-message-ids
// Uncomment this autofixer to grandfather in existing files.
Expand Down
10 changes: 9 additions & 1 deletion lib/rules/no-test-return-value.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const DEFAULT_TEST_HOOKS = [
'todo',
];

/** @type {import('eslint').Rule.RuleModule} */
/** @type {import('eslint').Rule.RuleModule & { DEFAULT_TEST_HOOKS: string[] }} */
module.exports = {
DEFAULT_TEST_HOOKS,
meta: {
Expand Down Expand Up @@ -80,6 +80,11 @@ module.exports = {
{
messageId: 'suggest',
fix(fixer) {
if (!node.range || !node.argument || !node.argument.range) {
throw new Error(
'This is just to make TypeScript happy. Every node should have a range and we already checked that the node has an argument.'
);
}
return fixer.removeRange([
node.range[0],
node.argument.range[0],
Expand All @@ -94,6 +99,9 @@ module.exports = {
},
};

/**
* @param {import('eslint').Rule.RuleContext} context
*/
function getTestHooks(context) {
return context.options && context.options[0] && context.options[0].testHooks;
}
37 changes: 26 additions & 11 deletions lib/rules/no-translation-key-interpolation.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

const isIdentifier = require('../utils/is-identifier');
const isMemberExpression = require('../utils/is-member-expression');
const { getImportIdentifier } = require('../utils/import');

const DEFAULT_SERVICE_NAME = 'intl';
Expand Down Expand Up @@ -46,6 +44,7 @@ module.exports = {
context.options &&
context.options.length > 0 &&
context.options[0].enforceStringLiteralKeys;
/** @type {string|undefined} */
let importedTranslationMethodName;

return {
Expand All @@ -59,7 +58,8 @@ module.exports = {

CallExpression(node) {
if (
(isT(node, importedTranslationMethodName) ||
((importedTranslationMethodName &&
isT(node, importedTranslationMethodName)) ||
isIntlT(node, serviceName) ||
isThisIntlT(node, serviceName)) &&
(node.arguments.length === 1 || node.arguments.length === 2)
Expand All @@ -80,37 +80,52 @@ module.exports = {
},
};

/**
* @param {import('eslint').Rule.Node} node
* @param {string} importedTranslationMethodName
* @returns {boolean}
*/
function isT(node, importedTranslationMethodName) {
// Example: import { t } from 'intl'; t(...);
return (
node.type === 'CallExpression' &&
isIdentifier(node.callee) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this form of comments not support node is Identifier? That would be really helpful for narrowing down what other things the node could have.

Copy link
Collaborator Author

@bmish bmish Aug 22, 2022

Choose a reason for hiding this comment

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

So yes I found out that that syntax is possible: https://github.com/open-wc/open-wc/blob/50e0ed588c67d516cb109f4106186265c239e218/packages/eslint-plugin-lit-a11y/lib/utils/aria.js#L25

But I've wanted to move away from these trivial helper functions for checking node types anyway. And removing unnecessary helper functions makes the code easier for TypeScript to understand in general.

node.callee.type === 'Identifier' &&
node.callee.name === importedTranslationMethodName
);
}

/**
* @param {import('eslint').Rule.Node} node
* @param {string} serviceName
* @returns {boolean}
*/
function isIntlT(node, serviceName) {
// Example: intl.t(...);
return (
node.type === 'CallExpression' &&
isMemberExpression(node.callee) &&
isIdentifier(node.callee.object) &&
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'Identifier' &&
node.callee.object.name === serviceName &&
isIdentifier(node.callee.property) &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === DEFAULT_METHOD_NAME
);
}

/**
* @param {import('eslint').Rule.Node} node
* @param {string} serviceName
* @returns {boolean}
*/
function isThisIntlT(node, serviceName) {
// Example: this.intl.t(...);
return (
node.type === 'CallExpression' &&
isMemberExpression(node.callee) &&
isMemberExpression(node.callee.object) &&
node.callee.type === 'MemberExpression' &&
node.callee.object.type === 'MemberExpression' &&
node.callee.object.object.type === 'ThisExpression' &&
isIdentifier(node.callee.object.property) &&
node.callee.object.property.type === 'Identifier' &&
node.callee.object.property.name === serviceName &&
isIdentifier(node.callee.property) &&
node.callee.property.type === 'Identifier' &&
node.callee.property.name === DEFAULT_METHOD_NAME
);
}
20 changes: 8 additions & 12 deletions lib/rules/require-await-function.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
'use strict';

const isIdentifier = require('../utils/is-identifier');
const isMemberExpression = require('../utils/is-member-expression');

/**
* Checks if the given node is part of a call with the `await` keyword.
*
* @param {ASTNode} node - the node to check.
* @param {import('eslint').Rule.Node} node - the node to check.
* @returns {boolean} `true` if the node is part of a call with the `await` keyword.
*/
function isAwaitCall(node) {
Expand All @@ -16,7 +13,7 @@ function isAwaitCall(node) {
return true;
}

if (parent.type === 'CallExpression' || isMemberExpression(parent)) {
if (parent.type === 'CallExpression' || parent.type === 'MemberExpression') {
// Check to see if the AwaitExpression is still another level above.
return isAwaitCall(parent);
}
Expand All @@ -27,7 +24,7 @@ function isAwaitCall(node) {
/**
* Checks if the given node is part of a call with the `return` keyword or direct arrow return.
*
* @param {ASTNode} node - the node to check.
* @param {import('eslint').Rule.Node} node - the node to check.
* @returns {boolean} `true` if the node is part of a returned call
*/
function isReturnCall(node) {
Expand All @@ -40,7 +37,7 @@ function isReturnCall(node) {
return true;
}

if (parent.type === 'CallExpression' || isMemberExpression(parent)) {
if (parent.type === 'CallExpression' || parent.type === 'MemberExpression') {
return isReturnCall(parent);
}

Expand All @@ -50,7 +47,7 @@ function isReturnCall(node) {
/**
* Checks if the given node is a nested call
*
* @param {ASTNode} node - the node to check.
* @param {import('eslint').Rule.Node} node - the node to check.
* @returns {boolean} `true` if the node is nested within another function
*/
function isNestedCall(node, isPromiseChain = false) {
Expand All @@ -64,7 +61,7 @@ function isNestedCall(node, isPromiseChain = false) {
}
}

if (isMemberExpression(parent)) {
if (parent.type === 'MemberExpression') {
return isNestedCall(parent, true);
}

Expand Down Expand Up @@ -102,10 +99,9 @@ module.exports = {
create(context) {
return {
CallExpression(node) {
const callee = node.callee;
if (
!isIdentifier(callee) ||
!context.options[0].functions.includes(callee.name)
node.callee.type !== 'Identifier' ||
!context.options[0].functions.includes(node.callee.name)
) {
// Not one of the specified async functions.
return;
Expand Down
41 changes: 29 additions & 12 deletions lib/rules/use-call-count-test-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const STUB_PROPERTY_NAMES = [
'called',
];

/** @type {import('eslint').Rule.RuleModule} */
/** @type {import('eslint').Rule.RuleModule & { ASSERT_PROPERTY_NAMES: string[], STUB_PROPERTY_NAMES: string[] }} */
module.exports = {
ASSERT_PROPERTY_NAMES,
STUB_PROPERTY_NAMES,
Expand All @@ -39,28 +39,38 @@ module.exports = {
return {
CallExpression(node) {
if (
!node.callee ||
!node.callee.object ||
node.callee.type !== 'MemberExpression' ||
node.callee.object.type !== 'Identifier' ||
node.callee.object.name !== 'assert'
) {
return;
}

const assertPropertyName = node.callee.property
? node.callee.property.name
: null;
if (!ASSERT_PROPERTY_NAMES.includes(assertPropertyName)) {
const assertPropertyName =
node.callee.type === 'MemberExpression' &&
node.callee.property.type === 'Identifier'
? node.callee.property.name
: null;
if (
!assertPropertyName ||
!ASSERT_PROPERTY_NAMES.includes(assertPropertyName)
) {
return;
}

if (node.arguments.length === 0) {
return;
}

const stubPropertyName = node.arguments[0].property
? node.arguments[0].property.name
: null;
if (!STUB_PROPERTY_NAMES.includes(stubPropertyName)) {
const stubPropertyName =
node.arguments[0].type === 'MemberExpression' &&
node.arguments[0].property.type === 'Identifier'
? node.arguments[0].property.name
: null;
if (
!stubPropertyName ||
!STUB_PROPERTY_NAMES.includes(stubPropertyName)
) {
return;
}

Expand All @@ -81,6 +91,13 @@ module.exports = {
const text = sourceCode.getText(node);

// Get `this.myStub` out of the `this.myStub.calledOnce` parameter.
if (
node.arguments[0].type !== 'MemberExpression' ||
!node.arguments[0].object.range ||
!node.range
) {
return null;
}
const stub = text.slice(
node.arguments[0].object.range[0] - node.range[0],
node.arguments[0].object.range[1] - node.range[0]
Expand All @@ -93,7 +110,7 @@ module.exports = {

// Retrieve the optional message parameter.
const optionalMessageParameter =
node.arguments.length === 2
node.arguments.length === 2 && node.arguments[1].range
? text.slice(
node.arguments[1].range[0] - node.range[0],
node.arguments[1].range[1] - node.range[0]
Expand Down
Loading