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 rules from eslint's recommended set that triggered good lints #50422

Merged
merged 9 commits into from Sep 19, 2022
Merged
12 changes: 10 additions & 2 deletions .eslintrc.json
Expand Up @@ -2,7 +2,6 @@
"parser": "@typescript-eslint/parser",
"parserOptions": {
"warnOnUnsupportedTypeScriptVersion": false,
"ecmaVersion": 6,
Copy link
Member Author

Choose a reason for hiding this comment

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

We don't need to set this; the default of "latest" is fine because we compile our source and any new syntax will be downleveled. No need to make eslint complain about it.

"sourceType": "module"
},
"env": {
Expand All @@ -27,6 +26,7 @@
"rules": {
"@typescript-eslint/adjacent-overload-signatures": "error",
"@typescript-eslint/array-type": "error",
"@typescript-eslint/no-array-constructor": "error",

"brace-style": "off",
"@typescript-eslint/brace-style": ["error", "stroustrup", { "allowSingleLine": true }],
Expand Down Expand Up @@ -62,12 +62,14 @@
"@typescript-eslint/prefer-for-of": "error",
"@typescript-eslint/prefer-function-type": "error",
"@typescript-eslint/prefer-namespace-keyword": "error",
"@typescript-eslint/prefer-as-const": "error",

"quotes": "off",
"@typescript-eslint/quotes": ["error", "double", { "avoidEscape": true, "allowTemplateLiterals": true }],

"semi": "off",
"@typescript-eslint/semi": "error",
"@typescript-eslint/no-extra-semi": "error",

"space-before-function-paren": "off",
"@typescript-eslint/space-before-function-paren": ["error", {
Expand All @@ -80,6 +82,9 @@
"@typescript-eslint/type-annotation-spacing": "error",
"@typescript-eslint/unified-signatures": "error",

"@typescript-eslint/no-extra-non-null-assertion": "error",
"@typescript-eslint/no-non-null-asserted-optional-chain": "error",

// scripts/eslint/rules
"local/object-literal-surrounding-space": "error",
"local/no-type-assertion-whitespace": "error",
Expand Down Expand Up @@ -143,6 +148,9 @@
"quote-props": ["error", "consistent-as-needed"],
"space-in-parens": "error",
"unicode-bom": ["error", "never"],
"use-isnan": "error"
"use-isnan": "error",
"no-prototype-builtins": "error",
"no-self-assign": "error",
"no-dupe-else-if": "error"
}
}
2 changes: 1 addition & 1 deletion scripts/word2md.ts
Expand Up @@ -185,7 +185,7 @@ function convertDocumentToMarkdown(doc: Word.Document): string {

function setProperties(target: any, properties: any) {
for (const name in properties) {
if (properties.hasOwnProperty(name)) {
if (Object.prototype.hasOwnProperty.call(properties, name)) {
const value = properties[name];
if (typeof value === "object") {
setProperties(target[name], value);
Expand Down
9 changes: 6 additions & 3 deletions src/compiler/binder.ts
Expand Up @@ -3041,8 +3041,11 @@ namespace ts {
return;
}
const rootExpr = getLeftmostAccessExpression(node.left);
if (isIdentifier(rootExpr) && lookupSymbolForName(container, rootExpr.escapedText)!?.flags & SymbolFlags.Alias) {
jakebailey marked this conversation as resolved.
Show resolved Hide resolved
return;
if (isIdentifier(rootExpr)) {
const symbol = lookupSymbolForName(container, rootExpr.escapedText);
if (symbol && symbol.flags & SymbolFlags.Alias) {
return;
}
}
// Fix up parent pointers since we're going to use these nodes before we bind into them
setParent(node.left, node);
Expand Down Expand Up @@ -3074,7 +3077,7 @@ namespace ts {
}

function bindPotentiallyMissingNamespaces(namespaceSymbol: Symbol | undefined, entityName: BindableStaticNameExpression, isToplevel: boolean, isPrototypeProperty: boolean, containerIsClass: boolean) {
if (namespaceSymbol?.flags! & SymbolFlags.Alias) {
if (namespaceSymbol && namespaceSymbol.flags & SymbolFlags.Alias) {
return namespaceSymbol;
}
if (isToplevel && !isPrototypeProperty) {
Expand Down
8 changes: 4 additions & 4 deletions src/compiler/checker.ts
Expand Up @@ -5837,8 +5837,8 @@ namespace ts {
}

function preserveCommentsOn<T extends Node>(node: T) {
if (some(propertySymbol.declarations, d => d.kind === SyntaxKind.JSDocPropertyTag)) {
const d = propertySymbol.declarations?.find(d => d.kind === SyntaxKind.JSDocPropertyTag)! as JSDocPropertyTag;
const d = propertySymbol.declarations?.find(d => d.kind === SyntaxKind.JSDocPropertyTag) as JSDocPropertyTag | undefined;
if (d) {
const commentText = getTextOfJSDocComment(d.comment);
if (commentText) {
setSyntheticLeadingComments(node, [{ kind: SyntaxKind.MultiLineCommentTrivia, text: "*\n * " + commentText.replace(/\n/g, "\n * ") + "\n ", pos: -1, end: -1, hasTrailingNewLine: true }]);
Expand Down Expand Up @@ -9198,7 +9198,7 @@ namespace ts {
if (container && (container.kind === SyntaxKind.Constructor || isJSConstructor(container))) {
return container as ConstructorDeclaration;
}
};
Copy link
Member

Choose a reason for hiding this comment

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

❤️ semicolons

}
}

/** Create a synthetic property access flow node after the last statement of the file */
Expand Down Expand Up @@ -29739,7 +29739,7 @@ namespace ts {
return !!s && getMinArgumentCount(s) >= 1 && isTypeAssignableTo(keyedType, getTypeAtPosition(s, 0));
}
return false;
};
}

const suggestedMethod = isAssignmentTarget(expr) ? "set" : "get";
if (!hasProp(suggestedMethod)) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/commandLineParser.ts
Expand Up @@ -3705,7 +3705,7 @@ namespace ts {
export function convertCompilerOptionsForTelemetry(opts: CompilerOptions): CompilerOptions {
const out: CompilerOptions = {};
for (const key in opts) {
if (opts.hasOwnProperty(key)) {
if (hasProperty(opts, key)) {
const type = getOptionFromName(key);
if (type !== undefined) { // Ignore unknown options
out[key] = getOptionValueWithEmptyStrings(opts[key], type);
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/debug.ts
Expand Up @@ -279,7 +279,7 @@ namespace ts {
if (typeof func !== "function") {
return "";
}
else if (func.hasOwnProperty("name")) {
else if (hasProperty(func, "name")) {
return (func as any).name;
}
else {
Expand Down Expand Up @@ -625,7 +625,7 @@ namespace ts {
];

for (const ctor of nodeConstructors) {
if (!ctor.prototype.hasOwnProperty("__debugKind")) {
if (!hasProperty(ctor, "__debugKind")) {
Object.defineProperties(ctor.prototype, {
// for use with vscode-js-debug's new customDescriptionGenerator in launch.json
__tsDebuggerDisplay: {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/factory/nodeFactory.ts
Expand Up @@ -5572,7 +5572,7 @@ namespace ts {
setOriginalNode(clone, node);

for (const key in node) {
if (clone.hasOwnProperty(key) || !node.hasOwnProperty(key)) {
if (hasProperty(clone, key) || !hasProperty(node, key)) {
continue;
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
Expand Up @@ -739,7 +739,7 @@ namespace ts {
return visitNodes(cbNode, cbNodes, node.typeParameters) ||
visitNodes(cbNode, cbNodes, node.parameters) ||
visitNode(cbNode, node.type);
};
}

function forEachChildInUnionOrIntersectionType<T>(node: UnionTypeNode | IntersectionTypeNode, cbNode: (node: Node) => T | undefined, cbNodes?: (nodes: NodeArray<Node>) => T | undefined): T | undefined {
return visitNodes(cbNode, cbNodes, node.types);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/program.ts
Expand Up @@ -503,7 +503,7 @@ namespace ts {
/* @internal */ imports: SourceFile["imports"];
/* @internal */ moduleAugmentations: SourceFile["moduleAugmentations"];
impliedNodeFormat?: SourceFile["impliedNodeFormat"];
};
}

/**
* Calculates the resulting resolution mode for some reference in some file - this is generally the explicitly
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scanner.ts
Expand Up @@ -359,7 +359,7 @@ namespace ts {

/* @internal */
export function computeLineStarts(text: string): number[] {
const result: number[] = new Array();
const result: number[] = [];
Copy link
Member

Choose a reason for hiding this comment

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

Maybe there was some reason for using new Array here? But performance doesn't seem to have changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. It's been there since the first commit of the TypeScript compiler.

let pos = 0;
let lineStart = 0;
while (pos < text.length) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/tracing.ts
Expand Up @@ -25,7 +25,7 @@ namespace ts { // eslint-disable-line local/one-namespace-per-file
// The actual constraint is that JSON.stringify be able to serialize it without throwing.
interface Args {
[key: string]: string | number | boolean | null | undefined | Args | readonly (string | number | boolean | null | undefined | Args)[];
};
}

/** Starts tracing for the given project. */
export function startTracing(tracingMode: Mode, traceDir: string, configFilePath?: string) {
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/transformers/declarations.ts
Expand Up @@ -218,9 +218,9 @@ namespace ts {
}

function reportNonlocalAugmentation(containingFile: SourceFile, parentSymbol: Symbol, symbol: Symbol) {
const primaryDeclaration = parentSymbol.declarations?.find(d => getSourceFileOfNode(d) === containingFile)!;
const primaryDeclaration = parentSymbol.declarations?.find(d => getSourceFileOfNode(d) === containingFile);
const augmentingDeclarations = filter(symbol.declarations, d => getSourceFileOfNode(d) !== containingFile);
if (augmentingDeclarations) {
if (primaryDeclaration && augmentingDeclarations) {
for (const augmentations of augmentingDeclarations) {
context.addDiagnostic(addRelatedInfo(
createDiagnosticForNode(augmentations, Diagnostics.Declaration_augments_declaration_in_another_file_This_cannot_be_serialized),
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/tsbuildPublic.ts
Expand Up @@ -487,7 +487,7 @@ namespace ts {
// TODO(rbuckton): Should be a `Set`, but that requires changing the code below that uses `mutateMapSkippingNewValues`
const currentProjects = new Map(
getBuildOrderFromAnyBuildOrder(buildOrder).map(
resolved => [toResolvedConfigFilePath(state, resolved), true as true])
resolved => [toResolvedConfigFilePath(state, resolved), true as const])
);

const noopOnDelete = { onDeleteValue: noop };
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/utilities.ts
Expand Up @@ -563,7 +563,7 @@ namespace ts {

interface ScriptTargetFeatures {
[key: string]: { [key: string]: string[] | undefined };
};
}

export function getScriptTargetFeatures(): ScriptTargetFeatures {
return {
Expand Down Expand Up @@ -5548,7 +5548,7 @@ namespace ts {

export function isWatchSet(options: CompilerOptions) {
// Firefox has Object.prototype.watch
return options.watch && options.hasOwnProperty("watch");
return options.watch && hasProperty(options, "watch");
}

export function closeFileWatcher(watcher: FileWatcher) {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilitiesPublic.ts
Expand Up @@ -1121,7 +1121,7 @@ namespace ts {

/* @internal */
export function isNodeArray<T extends Node>(array: readonly T[]): array is NodeArray<T> {
return array.hasOwnProperty("pos") && array.hasOwnProperty("end");
return hasProperty(array, "pos") && hasProperty(array, "end");
}

// Literals
Expand Down
2 changes: 1 addition & 1 deletion src/executeCommandLine/executeCommandLine.ts
Expand Up @@ -431,7 +431,7 @@ namespace ts {
function getHeader(sys: System, message: string) {
const colors = createColors(sys);
const header: string[] = [];
const terminalWidth = sys.getWidthOfTerminal?.() ?? 0;;
const terminalWidth = sys.getWidthOfTerminal?.() ?? 0;
const tsIconLength = 5;

const tsIconFirstLine = colors.blueBackground(padLeft("", tsIconLength));
Expand Down
2 changes: 1 addition & 1 deletion src/harness/fourslashImpl.ts
Expand Up @@ -2737,7 +2737,7 @@ namespace FourSlash {
const identifier = this.classificationToIdentifier(a.classificationType as number);
const text = this.activeFile.content.slice(a.textSpan.start, a.textSpan.start + a.textSpan.length);
replacement.push(` c2.semanticToken("${identifier}", "${text}"), `);
};
}
replacement.push(");");

throw new Error("You need to change the source code of fourslash test to use replaceWithSemanticClassifications");
Expand Down
4 changes: 2 additions & 2 deletions src/harness/fourslashInterfaceImpl.ts
Expand Up @@ -1895,11 +1895,11 @@ namespace FourSlashInterface {
};
export interface DiagnosticIgnoredInterpolations {
template: string
};
}
export type RenameLocationOptions = FourSlash.Range | { readonly range: FourSlash.Range, readonly prefixText?: string, readonly suffixText?: string };
export interface RenameOptions {
readonly findInStrings?: boolean;
readonly findInComments?: boolean;
readonly providePrefixAndSuffixTextForRename?: boolean;
};
}
}
4 changes: 2 additions & 2 deletions src/harness/harnessIO.ts
Expand Up @@ -335,7 +335,7 @@ namespace Harness {

export function setCompilerOptionsFromHarnessSetting(settings: TestCaseParser.CompilerSettings, options: ts.CompilerOptions & HarnessOptions): void {
for (const name in settings) {
if (settings.hasOwnProperty(name)) {
if (ts.hasProperty(settings, name)) {
const value = settings[name];
if (value === undefined) {
throw new Error(`Cannot have undefined value for compiler option '${name}'.`);
Expand Down Expand Up @@ -1496,7 +1496,7 @@ namespace Harness {

export function getConfigNameFromFileName(filename: string): "tsconfig.json" | "jsconfig.json" | undefined {
const flc = ts.getBaseFileName(filename).toLowerCase();
return ts.find(["tsconfig.json" as "tsconfig.json", "jsconfig.json" as "jsconfig.json"], x => x === flc);
return ts.find(["tsconfig.json" as const, "jsconfig.json" as const], x => x === flc);
}

if (Error) (Error as any).stackTraceLimit = 100;
Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessUtils.ts
Expand Up @@ -314,7 +314,7 @@ namespace Utils {

function findChildName(parent: any, child: any) {
for (const name in parent) {
if (parent.hasOwnProperty(name) && parent[name] === child) {
if (ts.hasProperty(parent, name) && parent[name] === child) {
return name;
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/harness/vfsUtil.ts
Expand Up @@ -916,7 +916,6 @@ namespace vfs {
if (this._shadowRoot) {
this._copyShadowLinks(this._shadowRoot._getRootLinks(), this._lazy.links);
}
this._lazy.links = this._lazy.links;
}
return this._lazy.links;
}
Expand Down Expand Up @@ -1578,7 +1577,7 @@ namespace vfs {
const entry = normalizeFileSetEntry(container[name]);
const file = dirname ? vpath.combine(dirname, name) : name;
// eslint-disable-next-line no-null/no-null
if (entry === null || entry === undefined || entry instanceof Unlink || entry instanceof Rmdir) {
if (entry === null || entry === undefined || entry instanceof Unlink) {
text += `//// [${file}] unlink\r\n`;
}
else if (entry instanceof Rmdir) {
Expand Down
2 changes: 1 addition & 1 deletion src/loggedIO/loggedIO.ts
Expand Up @@ -94,7 +94,7 @@ namespace Playback { // eslint-disable-line local/one-namespace-per-file
function memoize<T>(func: (s: string) => T): Memoized<T> {
let lookup: { [s: string]: T } = {};
const run: Memoized<T> = ((s: string) => {
if (lookup.hasOwnProperty(s)) return lookup[s];
if (ts.hasProperty(lookup, s)) return lookup[s];
return lookup[s] = func(s);
}) as Memoized<T>;
run.reset = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/server/editorServices.ts
Expand Up @@ -940,7 +940,7 @@ namespace ts.server {
// raw is now fixed and ready
this.safelist = raw.typesMap;
for (const key in raw.simpleMap) {
if (raw.simpleMap.hasOwnProperty(key)) {
if (hasProperty(raw.simpleMap, key)) {
this.legacySafelist.set(key, raw.simpleMap[key].toLowerCase());
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Expand Up @@ -297,7 +297,7 @@ namespace ts.server {
interface ProjectNavigateToItems {
project: Project;
navigateToItems: readonly NavigateToItem[];
};
}

function createDocumentSpanSet(): Set<DocumentSpan> {
return createSet(({textSpan}) => textSpan.start + 100003 * textSpan.length, documentSpansEqual);
Expand Down
4 changes: 2 additions & 2 deletions src/services/completions.ts
Expand Up @@ -1107,7 +1107,7 @@ namespace ts.Completions {

return { isSnippet, insertText, labelDetails };

};
}

function createObjectLiteralMethod(
symbol: Symbol,
Expand Down Expand Up @@ -4328,7 +4328,7 @@ namespace ts.Completions {

function isArrowFunctionBody(node: Node) {
return node.parent && isArrowFunction(node.parent) && node.parent.body === node;
};
}

/** True if symbol is a type or a module containing at least one type. */
function symbolCanBeReferencedAtTypeLocation(symbol: Symbol, checker: TypeChecker, seenModules = new Map<SymbolId, true>()): boolean {
Expand Down
10 changes: 5 additions & 5 deletions src/services/formatting/rules.ts
Expand Up @@ -410,23 +410,23 @@ namespace ts.formatting {
}

function isOptionEnabled(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
return (context) => context.options && context.options.hasOwnProperty(optionName) && !!context.options[optionName];
return (context) => context.options && hasProperty(context.options, optionName) && !!context.options[optionName];
}

function isOptionDisabled(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
return (context) => context.options && context.options.hasOwnProperty(optionName) && !context.options[optionName];
return (context) => context.options && hasProperty(context.options, optionName) && !context.options[optionName];
}

function isOptionDisabledOrUndefined(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName];
return (context) => !context.options || !hasProperty(context.options, optionName) || !context.options[optionName];
}

function isOptionDisabledOrUndefinedOrTokensOnSameLine(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName] || context.TokensAreOnSameLine();
return (context) => !context.options || !hasProperty(context.options, optionName) || !context.options[optionName] || context.TokensAreOnSameLine();
}

function isOptionEnabledOrUndefined(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean {
return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !!context.options[optionName];
return (context) => !context.options || !hasProperty(context.options, optionName) || !!context.options[optionName];
}

function isForContext(context: FormattingContext): boolean {
Expand Down
2 changes: 1 addition & 1 deletion src/services/refactors/convertExport.ts
Expand Up @@ -54,7 +54,7 @@ namespace ts.refactor {
readonly exportName: Identifier; // This is exportNode.name except for VariableStatement_s.
readonly wasDefault: boolean;
readonly exportingModuleSymbol: Symbol;
};
}

function getInfo(context: RefactorContext, considerPartialSpans = true): ExportInfo | RefactorErrorInfo | undefined {
const { file, program } = context;
Expand Down