Skip to content

Commit

Permalink
chore: fix miscellaneous no-unnecessary-condition violations (#7834)
Browse files Browse the repository at this point in the history
* chore: fix miscellaneous no-unnecessary-condition violations

* Fixed references to deleted getTypeArguments function, and unused disables

* Add getTypeArguments back, deprecated
  • Loading branch information
JoshuaKGoldberg committed Nov 13, 2023
1 parent 72cb9e4 commit 8e87fac
Show file tree
Hide file tree
Showing 23 changed files with 48 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@ This rule simply warns against using them, as using them will likely introduce t
*/

const BANNED_PROPERTIES = [
// {
// type: 'Node',
// property: 'parent',
// fixWith: null,
// },
{
type: 'Symbol',
property: 'declarations',
Expand Down Expand Up @@ -87,10 +82,6 @@ export default createRule({
messageId: 'suggestedFix',
data: banned,
fix(fixer): TSESLint.RuleFix | null {
if (banned.fixWith == null) {
return null;
}

return fixer.replaceText(node.property, banned.fixWith);
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ export default createRule({
return {
Literal(node: TSESTree.Literal): void {
if (
node.parent?.type === AST_NODE_TYPES.TSEnumMember &&
node.parent.parent?.type === AST_NODE_TYPES.TSEnumDeclaration &&
node.parent.type === AST_NODE_TYPES.TSEnumMember &&
node.parent.parent.type === AST_NODE_TYPES.TSEnumDeclaration &&
['AST_NODE_TYPES', 'AST_TOKEN_TYPES', 'DefinitionType'].includes(
node.parent.parent.id.name,
)
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-tslint/src/rules/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export default createRule<Options, MessageIds>({
/**
* Format the TSLint results for ESLint
*/
if (result.failures?.length) {
if (result.failures.length) {
result.failures.forEach(failure => {
const start = failure.getStartPosition().getLineAndCharacter();
const end = failure.getEndPosition().getLineAndCharacter();
Expand Down
6 changes: 3 additions & 3 deletions packages/eslint-plugin/src/rules/no-misused-promises.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { AST_NODE_TYPES } from '@typescript-eslint/utils';
import * as tsutils from 'ts-api-utils';
import * as ts from 'typescript';

import { createRule, getParserServices, getTypeArguments } from '../util';
import { createRule, getParserServices } from '../util';

type Options = [
{
Expand Down Expand Up @@ -556,7 +556,7 @@ function voidFunctionArguments(
// Unwrap 'Array<MaybeVoidFunction>' to 'MaybeVoidFunction',
// so that we'll handle it in the same way as a non-rest
// 'param: MaybeVoidFunction'
type = getTypeArguments(type, checker)[0];
type = checker.getTypeArguments(type)[0];
for (let i = index; i < node.arguments.length; i++) {
checkThenableOrVoidArgument(
checker,
Expand All @@ -570,7 +570,7 @@ function voidFunctionArguments(
} else if (checker.isTupleType(type)) {
// Check each type in the tuple - for example, [boolean, () => void] would
// add the index of the second tuple parameter to 'voidReturnIndices'
const typeArgs = getTypeArguments(type, checker);
const typeArgs = checker.getTypeArguments(type);
for (
let i = index;
i < node.arguments.length && i - index < typeArgs.length;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
createRule,
findFirstResult,
getParserServices,
getTypeArguments,
isTypeReferenceType,
} from '../util';

Expand Down Expand Up @@ -51,7 +50,7 @@ export default createRule<[], MessageIds>({
if (isTypeReferenceType(type)) {
return {
type: type.target,
typeArguments: getTypeArguments(type, checker),
typeArguments: checker.getTypeArguments(type),
};
}
return {
Expand Down
11 changes: 4 additions & 7 deletions packages/eslint-plugin/src/rules/no-unsafe-argument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as ts from 'typescript';
import {
createRule,
getParserServices,
getTypeArguments,
isTypeAnyArrayType,
isTypeAnyType,
isUnsafeAssignment,
Expand Down Expand Up @@ -64,13 +63,13 @@ class FunctionSignature {
// is a rest param
if (checker.isArrayType(type)) {
restType = {
type: getTypeArguments(type, checker)[0],
type: checker.getTypeArguments(type)[0],
kind: RestTypeKind.Array,
index: i,
};
} else if (checker.isTupleType(type)) {
restType = {
typeArguments: getTypeArguments(type, checker),
typeArguments: checker.getTypeArguments(type),
kind: RestTypeKind.Tuple,
index: i,
};
Expand Down Expand Up @@ -205,10 +204,8 @@ export default createRule<[], MessageIds>({
});
} else if (checker.isTupleType(spreadArgType)) {
// foo(...[tuple1, tuple2])
const spreadTypeArguments = getTypeArguments(
spreadArgType,
checker,
);
const spreadTypeArguments =
checker.getTypeArguments(spreadArgType);
for (const tupleType of spreadTypeArguments) {
const parameterType = signature.getNextParameterType();
if (parameterType == null) {
Expand Down
3 changes: 1 addition & 2 deletions packages/eslint-plugin/src/rules/no-unsafe-assignment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
getContextualType,
getParserServices,
getThisExpression,
getTypeArguments,
isTypeAnyArrayType,
isTypeAnyType,
isTypeUnknownType,
Expand Down Expand Up @@ -97,7 +96,7 @@ export default createRule({
return true;
}

const tupleElements = getTypeArguments(senderType, checker);
const tupleElements = checker.getTypeArguments(senderType);

// tuple with any
// const [x] = [1 as any];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
createRule,
getConstrainedTypeAtLocation,
getParserServices,
getTypeArguments,
getTypeName,
isTypeArrayTypeOrUnionOfArrayTypes,
} from '../util';
Expand Down Expand Up @@ -61,7 +60,7 @@ export default createRule<Options, MessageIds>({
const type = services.getTypeAtLocation(node);

if (checker.isArrayType(type) || checker.isTupleType(type)) {
const typeArgs = getTypeArguments(type, checker);
const typeArgs = checker.getTypeArguments(type);
return typeArgs.every(arg => getTypeName(checker, arg) === 'string');
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ export function generateArrayType(
// but that's obviously dumb and loose so let's not even bother with it
throw new UnexpectedError('Unexpected missing items', schema);
}
if (
schema.items &&
!TSUtils.isArray(schema.items) &&
schema.additionalItems
) {
if (!TSUtils.isArray(schema.items) && schema.additionalItems) {
throw new NotSupportedError(
'singlely-typed array with additionalItems',
schema,
Expand Down
8 changes: 5 additions & 3 deletions packages/rule-tester/src/utils/config-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const { ConfigOps, environments: BuiltInEnvironments } = Legacy;
const ajv = ajvBuilder();
const ruleValidators = new WeakMap<AnyRuleModule, ValidateFunction>();

let validateSchema: ValidateFunction;
let validateSchema: ValidateFunction | undefined;
const severityMap = {
error: 2,
warn: 1,
Expand All @@ -40,8 +40,10 @@ function validateRuleSeverity(options: Linter.RuleEntry): number | string {
const severity = Array.isArray(options) ? options[0] : options;
const normSeverity =
typeof severity === 'string'
? severityMap[severity.toLowerCase() as Linter.SeverityString]
: severity;
? (severityMap[severity.toLowerCase() as Linter.SeverityString] as
| number
| undefined)
: (severity as number);

if (normSeverity === 0 || normSeverity === 1 || normSeverity === 2) {
return normSeverity;
Expand Down
4 changes: 3 additions & 1 deletion packages/rule-tester/src/utils/getRuleOptionsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { isReadonlyArray } from './isReadonlyArray';
* @param rule A new-style rule object
* @returns JSON Schema for the rule's options.
*/
export function getRuleOptionsSchema(rule: AnyRuleModule): JSONSchema4 | null {
export function getRuleOptionsSchema(
rule: Partial<AnyRuleModule>,
): JSONSchema4 | null {
const schema = rule.meta?.schema;

// Given a tuple of schemas, insert warning level at the beginning
Expand Down
2 changes: 1 addition & 1 deletion packages/rule-tester/src/utils/interpolate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { ReportDescriptorMessageData } from '@typescript-eslint/utils/ts-es

export function interpolate(
text: string,
data: ReportDescriptorMessageData,
data: ReportDescriptorMessageData | undefined,
): string {
if (!data) {
return text;
Expand Down
14 changes: 2 additions & 12 deletions packages/scope-manager/src/referencer/ClassVisitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ class ClassVisitor extends Visitor {
this.visitType(node.typeParameters);
// then the usages
this.visitType(node.superTypeArguments);
node.implements?.forEach(imp => this.visitType(imp));
node.implements.forEach(imp => this.visitType(imp));

this.visit(node.body);

Expand Down Expand Up @@ -224,17 +224,7 @@ class ClassVisitor extends Visitor {
this.visitMetadataType(node.returnType, withMethodDecorators);
this.visitType(node.typeParameters);

// In TypeScript there are a number of function-like constructs which have no body,
// so check it exists before traversing
if (node.body) {
// Skip BlockStatement to prevent creating BlockStatement scope.
if (node.body.type === AST_NODE_TYPES.BlockStatement) {
this.#referencer.visitChildren(node.body);
} else {
this.#referencer.visit(node.body);
}
}

this.#referencer.visitChildren(node.body);
this.#referencer.close(node);
}

Expand Down
5 changes: 2 additions & 3 deletions packages/scope-manager/src/referencer/PatternVisitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,11 @@ class PatternVisitor extends VisitorBase {
}

protected Identifier(pattern: TSESTree.Identifier): void {
const lastRestElement =
this.#restElements[this.#restElements.length - 1] ?? null;
const lastRestElement = this.#restElements.at(-1);

this.#callback(pattern, {
topLevel: pattern === this.#rootPattern,
rest: lastRestElement != null && lastRestElement.argument === pattern,
rest: lastRestElement?.argument === pattern,
assignments: this.#assignments,
});
}
Expand Down
6 changes: 3 additions & 3 deletions packages/scope-manager/src/referencer/TypeVisitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { TSESTree } from '@typescript-eslint/types';
import { AST_NODE_TYPES } from '@typescript-eslint/types';

import { ParameterDefinition, TypeDefinition } from '../definition';
import { ScopeType } from '../scope';
import { type Scope, ScopeType } from '../scope';
import type { Referencer } from './Referencer';
import { Visitor } from './Visitor';

Expand Down Expand Up @@ -147,7 +147,7 @@ class TypeVisitor extends Visitor {
scope.type === ScopeType.mappedType
) {
// search up the scope tree to figure out if we're in a nested type scope
let currentScope = scope.upper;
let currentScope = scope.upper as Scope | undefined;
while (currentScope) {
if (
currentScope.type === ScopeType.functionType ||
Expand Down Expand Up @@ -186,7 +186,7 @@ class TypeVisitor extends Visitor {
this.visit(node.typeParameters);
}

node.extends?.forEach(this.visit, this);
node.extends.forEach(this.visit, this);
this.visit(node.body);

if (node.typeParameters) {
Expand Down
2 changes: 1 addition & 1 deletion packages/scope-manager/src/scope/GlobalScope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class GlobalScope extends ScopeBase<
// create an implicit global variable from assignment expression
const info = ref.maybeImplicitGlobal;
const node = info.pattern;
if (node && node.type === AST_NODE_TYPES.Identifier) {
if (node.type === AST_NODE_TYPES.Identifier) {
this.defineVariable(
node.name,
this.implicit.set,
Expand Down
21 changes: 6 additions & 15 deletions packages/scope-manager/src/scope/ScopeBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,15 @@ abstract class ScopeBase<
block: TBlock,
isMethodDefinition: boolean,
) {
const upperScopeAsScopeBase = upperScope!;
const upperScopeAsScopeBase = upperScope;

this.type = type;
this.#dynamic =
this.type === ScopeType.global || this.type === ScopeType.with;
this.block = block;
this.variableScope = this.isVariableScope()
? this
: upperScopeAsScopeBase.variableScope;
: upperScopeAsScopeBase!.variableScope;
this.upper = upperScope;

/**
Expand All @@ -249,10 +249,8 @@ abstract class ScopeBase<
*/
this.isStrict = isStrictScope(this as Scope, block, isMethodDefinition);

if (upperScopeAsScopeBase) {
// this is guaranteed to be correct at runtime
upperScopeAsScopeBase.childScopes.push(this as Scope);
}
// this is guaranteed to be correct at runtime
upperScopeAsScopeBase?.childScopes.push(this as Scope);

this.#declaredVariables = scopeManager.declaredVariables;

Expand Down Expand Up @@ -293,11 +291,7 @@ abstract class ScopeBase<
return (
defs.length > 0 &&
defs.every(def => {
if (
def.type === DefinitionType.Variable &&
def.parent?.type === AST_NODE_TYPES.VariableDeclaration &&
def.parent.kind === 'var'
) {
if (def.type === DefinitionType.Variable && def.parent.kind === 'var') {
return false;
}
return true;
Expand Down Expand Up @@ -386,10 +380,7 @@ abstract class ScopeBase<
}

protected delegateToUpperScope(ref: Reference): void {
const upper = this.upper! as AnyScope;
if (upper?.leftToResolve) {
upper.leftToResolve.push(ref);
}
(this.upper as AnyScope | undefined)?.leftToResolve?.push(ref);
this.through.push(ref);
}

Expand Down
8 changes: 4 additions & 4 deletions packages/scope-manager/tests/types/reference-type.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('referencing a type - positive', () => {
AST_NODE_TYPES.TSTypeAliasDeclaration,
n => n.id.name === 'OtherType',
);
expect(variable.references[0].identifier.parent?.parent).toBe(
expect(variable.references[0].identifier.parent.parent).toBe(
referencingNode,
);
});
Expand All @@ -39,7 +39,7 @@ describe('referencing a type - positive', () => {
ast,
AST_NODE_TYPES.TSTypeAliasDeclaration,
);
expect(variable.references[0].identifier.parent?.parent).toBe(
expect(variable.references[0].identifier.parent.parent).toBe(
referencingNode,
);
});
Expand Down Expand Up @@ -87,7 +87,7 @@ describe('referencing a type - positive', () => {
AST_NODE_TYPES.TSTypeAliasDeclaration,
n => n.id.name === 'reference1',
);
expect(variable.references[1].identifier.parent?.parent).toBe(
expect(variable.references[1].identifier.parent.parent).toBe(
referencingTypeNode,
);
// third ref is the variable reference
Expand Down Expand Up @@ -141,7 +141,7 @@ describe('referencing a type - positive', () => {
ast,
AST_NODE_TYPES.TSTypeAliasDeclaration,
);
expect(variable.references[1].identifier.parent?.parent).toBe(
expect(variable.references[1].identifier.parent.parent).toBe(
referencingNode,
);
});
Expand Down
3 changes: 0 additions & 3 deletions packages/type-utils/src/getContextualType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ export function getContextualType(
node: ts.Expression,
): ts.Type | undefined {
const parent = node.parent;
if (!parent) {
return;
}

if (ts.isCallExpression(parent) || ts.isNewExpression(parent)) {
if (node === parent.expression) {
Expand Down
Loading

0 comments on commit 8e87fac

Please sign in to comment.