Skip to content

Commit

Permalink
Added some code improvements from a working branch: (#8822)
Browse files Browse the repository at this point in the history
* Added some code improvements from a working branch:
* Created a shortened "fileId" for each source file. This reduces memory used for tracking symbols and TypeVars that are scoped to files.
* Removed some unused or redundant code.
* Added assertNever calls to catch potential bugs.
* Renamed a few symbols for consistency and readability.
* Improved handling of literal type expressions that are lazily parsed.
* Refactored literal type printing into a utility module.

* Fixed style issue.
  • Loading branch information
erictraut authored Aug 24, 2024
1 parent b2d7ed5 commit 020dea1
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 66 deletions.
1 change: 1 addition & 0 deletions packages/pyright-internal/src/analyzer/analyzerFileInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export interface AnalyzerFileInfo {
lines: TextRangeCollection<TextRange>;
typingSymbolAliases: Map<string, string>;
definedConstants: Map<string, boolean | string>;
fileId: string;
fileUri: Uri;
moduleName: string;
isStubFile: boolean;
Expand Down
10 changes: 1 addition & 9 deletions packages/pyright-internal/src/analyzer/constraintSolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,8 @@ export class ConstraintSolutionSet {
// Indexed by TypeVar ID.
private _typeVarMap: Map<string, Type | undefined>;

// See the comment in constraintTracker for details about identifying
// solution sets by scope ID.
private _scopeIds: Set<string> | undefined;

constructor(scopeIds?: Set<string>) {
constructor() {
this._typeVarMap = new Map<string, Type>();

if (scopeIds) {
this._scopeIds = new Set<string>(scopeIds);
}
}

isEmpty() {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/analyzer/constraintSolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ export function solveConstraintSet(
constraintSet: ConstraintSet,
options?: SolveConstraintsOptions
): ConstraintSolutionSet {
const solutionSet = new ConstraintSolutionSet(constraintSet.getScopeIds());
const solutionSet = new ConstraintSolutionSet();

constraintSet.doForEachTypeVar((entry) => {
solveTypeVarRecursive(evaluator, constraintSet, options, solutionSet, entry);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface TypeVarConstraints {
// Records the constraints information for a set of type variables
// associated with a callee's signature.
export class ConstraintSet {
// Maps type variable IDs to their current constraints.
private _typeVarMap: Map<string, TypeVarConstraints>;

// A set of one or more TypeVar scope IDs that identify this constraint set.
Expand Down
11 changes: 10 additions & 1 deletion packages/pyright-internal/src/analyzer/declarationUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Collection of static methods that operate on declarations.
*/

import { assertNever } from '../common/debug';
import { getEmptyRange } from '../common/textRange';
import { Uri } from '../common/uri/uri';
import { NameNode, ParseNodeType } from '../parser/parseNodes';
Expand Down Expand Up @@ -138,6 +139,10 @@ export function getNameFromDeclaration(declaration: Declaration) {
declaration.node.d.valueExpr.nodeType === ParseNodeType.Name
? declaration.node.d.valueExpr.d.value
: undefined;

default: {
assertNever(declaration);
}
}

throw new Error(`Shouldn't reach here`);
Expand Down Expand Up @@ -167,6 +172,10 @@ export function getNameNodeForDeclaration(declaration: Declaration): NameNode |
case DeclarationType.Intrinsic:
case DeclarationType.SpecialBuiltInClass:
return undefined;

default: {
assertNever(declaration);
}
}

throw new Error(`Shouldn't reach here`);
Expand Down Expand Up @@ -197,7 +206,7 @@ export function getDeclarationsWithUsesLocalNameRemoved(decls: Declaration[]) {
});
}

export function createSynthesizedAliasDeclaration(uri: Uri): AliasDeclaration {
export function synthesizeAliasDeclaration(uri: Uri): AliasDeclaration {
// The only time this decl is used is for IDE services such as
// the find all references, hover provider and etc.
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/pyright-internal/src/analyzer/parseTreeUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2876,7 +2876,7 @@ export function getScopeIdForNode(node: ParseNode): string {
}

const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
return `${fileInfo.fileUri.key}.${node.start.toString()}-${name}`;
return `${fileInfo.fileId}.${node.start.toString()}-${name}`;
}

// Walks up the parse tree and finds all scopes that can provide
Expand Down
10 changes: 5 additions & 5 deletions packages/pyright-internal/src/analyzer/parseTreeWalker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -795,23 +795,23 @@ export class ParseTreeVisitor<T> {
return this._default;
}

visitPatternMappingExpandEntry(node: PatternMappingExpandEntryNode) {
visitPatternMapping(node: PatternMappingNode) {
return this._default;
}

visitPatternSequence(node: PatternSequenceNode) {
visitPatternMappingExpandEntry(node: PatternMappingExpandEntryNode) {
return this._default;
}

visitPatternValue(node: PatternValueNode) {
visitPatternMappingKeyEntry(node: PatternMappingKeyEntryNode) {
return this._default;
}

visitPatternMappingKeyEntry(node: PatternMappingKeyEntryNode) {
visitPatternSequence(node: PatternSequenceNode) {
return this._default;
}

visitPatternMapping(node: PatternMappingNode) {
visitPatternValue(node: PatternValueNode) {
return this._default;
}

Expand Down
30 changes: 30 additions & 0 deletions packages/pyright-internal/src/analyzer/sourceFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ export enum IPythonMode {
CellDocs,
}

// A monotonically increasing number used to create unique file IDs.
let nextUniqueFileId = 1;

class WriteableData {
// Number that is incremented every time the diagnostics
// are updated.
Expand Down Expand Up @@ -193,6 +196,10 @@ export class SourceFile {
// a real file on disk.
private readonly _uri: Uri;

// A short string that is guaranteed to uniquely
// identify this file.
private readonly _fileId: string;

// Period-delimited import path for the module.
private _moduleName: string;

Expand Down Expand Up @@ -258,6 +265,7 @@ export class SourceFile {

this._editMode = editMode;
this._uri = uri;
this._fileId = this._makeFileId(uri);
this._moduleName = moduleName;
this._isStubFile = uri.hasExtension('.pyi');
this._isThirdPartyImport = isThirdPartyImport;
Expand Down Expand Up @@ -954,6 +962,27 @@ export class SourceFile {
return new TextRangeDiagnosticSink(lines);
}

// Creates a short string that can be used to uniquely identify
// this file from all other files. It is used in the type evaluator
// to distinguish between types that are defined in different files
// or scopes.
private _makeFileId(uri: Uri) {
const maxNameLength = 8;

// Use a small portion of the file name to help with debugging.
let fileName = uri.fileNameWithoutExtensions;
if (fileName.length > maxNameLength) {
fileName = fileName.substring(fileName.length - maxNameLength);
}

// Append a number to guarantee uniqueness.
const uniqueNumber = nextUniqueFileId++;

// Use a "/" to separate the two components, since this
// character will never appear in a file name.
return `${fileName}/${uniqueNumber.toString()}`;
}

// Computes an updated set of accumulated diagnostics for the file
// based on the partial diagnostics from various analysis stages.
private _recomputeDiagnostics(configOptions: ConfigOptions) {
Expand Down Expand Up @@ -1302,6 +1331,7 @@ export class SourceFile {
lines: this._writableData.tokenizerLines!,
typingSymbolAliases: this._writableData.parserOutput!.typingSymbolAliases,
definedConstants: configOptions.defineConstant,
fileId: this._fileId,
fileUri: this._uri,
moduleName: this.getModuleName(),
isStubFile: this._isStubFile,
Expand Down
22 changes: 9 additions & 13 deletions packages/pyright-internal/src/analyzer/typeEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ import {
YieldFromNode,
YieldNode,
} from '../parser/parseNodes';
import { ParseOptions, ParseTextMode, Parser } from '../parser/parser';
import { ParseOptions, Parser, ParseTextMode } from '../parser/parser';
import { KeywordType, OperatorType, StringTokenFlags } from '../parser/tokenizerTypes';
import { AnalyzerFileInfo, ImportLookup, isAnnotationEvaluationPostponed } from './analyzerFileInfo';
import * as AnalyzerNodeInfo from './analyzerNodeInfo';
Expand Down Expand Up @@ -120,11 +120,11 @@ import {
VariableDeclaration,
} from './declaration';
import {
createSynthesizedAliasDeclaration,
getDeclarationsWithUsesLocalNameRemoved,
getNameNodeForDeclaration,
resolveAliasDeclaration as resolveAliasDeclarationUtil,
ResolvedAliasInfo,
synthesizeAliasDeclaration,
} from './declarationUtils';
import {
addOverloadsToFunctionType,
Expand Down Expand Up @@ -21070,7 +21070,7 @@ export function createTypeEvaluator(
// Synthesize an alias declaration for this name part. The only
// time this case is used is for IDE services such as
// the find all references, hover provider and etc.
declarations.push(createSynthesizedAliasDeclaration(importInfo.resolvedUris[namePartIndex]));
declarations.push(synthesizeAliasDeclaration(importInfo.resolvedUris[namePartIndex]));
}
}
} else if (node.parent && node.parent.nodeType === ParseNodeType.Argument && node === node.parent.d.name) {
Expand Down Expand Up @@ -23052,15 +23052,6 @@ export function createTypeEvaluator(
prevSrcType = curSrcType;
}

// If we're enforcing invariance, literal types must match as well.
if ((flags & AssignTypeFlags.Invariant) !== 0) {
const srcIsLiteral = srcType.priv.literalValue !== undefined;
const destIsLiteral = destType.priv.literalValue !== undefined;
if (srcIsLiteral !== destIsLiteral) {
return false;
}
}

// Handle tuple, which supports a variable number of type arguments.
if (destType.priv.tupleTypeArgs && curSrcType.priv.tupleTypeArgs) {
return assignTupleTypeArgs(
Expand Down Expand Up @@ -26958,11 +26949,16 @@ export function createTypeEvaluator(
);

if (parseResults.parseTree) {
const fileInfo = AnalyzerNodeInfo.getFileInfo(node);
parseResults.diagnostics.forEach((diag) => {
addDiagnosticWithSuppressionCheck('error', diag.message, node);
fileInfo.diagnosticSink.addDiagnosticWithTextRange('error', diag.message, node);
});

parseResults.parseTree.parent = node;

// Add the new subtree to the parse tree so it can participate in
// language server operations like find and replace.
node.d.annotation = parseResults.parseTree;
return parseResults.parseTree;
}

Expand Down
38 changes: 4 additions & 34 deletions packages/pyright-internal/src/analyzer/typePrinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { assert } from '../common/debug';
import { ParamCategory } from '../parser/parseNodes';
import { isTypedKwargs } from './parameterUtils';
import * as ParseTreeUtils from './parseTreeUtils';
import { printBytesLiteral, printStringLiteral } from './typePrinterUtils';
import {
ClassType,
EnumLiteral,
Expand Down Expand Up @@ -39,9 +40,6 @@ import {
} from './types';
import { convertToInstance, doForEachSubtype, isNoneInstance, isTupleClass, removeNoneFromUnion } from './typeUtils';

const singleTickRegEx = /'/g;
const escapedDoubleQuoteRegEx = /\\"/g;

export const enum PrintTypeFlags {
None = 0,

Expand Down Expand Up @@ -161,37 +159,9 @@ export function printLiteralValue(type: ClassType, quotation = "'"): string {
}

if (type.shared.name === 'bytes') {
let bytesString = '';

// There's no good built-in conversion routine in javascript to convert
// bytes strings. Determine on a character-by-character basis whether
// it can be rendered into an ASCII character. If not, use an escape.
for (let i = 0; i < effectiveLiteralValue.length; i++) {
const char = effectiveLiteralValue.substring(i, i + 1);
const charCode = char.charCodeAt(0);

if (charCode >= 20 && charCode <= 126) {
if (charCode === 34) {
bytesString += '\\' + char;
} else {
bytesString += char;
}
} else {
bytesString += `\\x${((charCode >> 4) & 0xf).toString(16)}${(charCode & 0xf).toString(16)}`;
}
}

literalStr = `b"${bytesString}"`;
literalStr = printBytesLiteral(effectiveLiteralValue);
} else {
// JSON.stringify will perform proper escaping for " case.
// So, we only need to do our own escaping for ' case.
literalStr = JSON.stringify(effectiveLiteralValue).toString();
if (quotation !== '"') {
literalStr = `'${literalStr
.substring(1, literalStr.length - 1)
.replace(escapedDoubleQuoteRegEx, '"')
.replace(singleTickRegEx, "\\'")}'`; // CodeQL [SM02383] Code ql is just wrong here. We don't need to replace backslashes.
}
literalStr = printStringLiteral(effectiveLiteralValue, quotation);
}
} else if (typeof literalValue === 'boolean') {
literalStr = literalValue ? 'True' : 'False';
Expand Down Expand Up @@ -523,7 +493,7 @@ function printTypeInternal(
case TypeCategory.Union: {
// If this is a value expression that evaluates to a union type but is
// not a type alias, simply print the special form ("UnionType").
if (TypeBase.isInstantiable(type) && type.props?.specialForm && !aliasInfo) {
if (TypeBase.isInstantiable(type) && type.props?.specialForm && !type.props?.typeAliasInfo) {
return printTypeInternal(
type.props.specialForm,
printTypeFlags,
Expand Down
49 changes: 49 additions & 0 deletions packages/pyright-internal/src/analyzer/typePrinterUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* typePrinterUtils.ts
* Copyright (c) Microsoft Corporation.
* Licensed under the MIT license.
* Author: Eric Traut
*
* Simple utility functions used by the type printer.
*/

export function printStringLiteral(value: string, quotation = '"'): string {
const singleTickRegEx = /'/g;
const escapedDoubleQuoteRegEx = /\\"/g;

// JSON.stringify will perform proper escaping for " case.
// So, we only need to do our own escaping for ' case.
let literalStr = JSON.stringify(value).toString();
if (quotation !== '"') {
literalStr = `'${literalStr
.substring(1, literalStr.length - 1)
.replace(escapedDoubleQuoteRegEx, '"')
.replace(singleTickRegEx, "\\'")}'`; // CodeQL [SM02383] Code ql is just wrong here. We don't need to replace backslashes.
}

return literalStr;
}

export function printBytesLiteral(value: string) {
let bytesString = '';

// There's no good built-in conversion routine in javascript to convert
// bytes strings. Determine on a character-by-character basis whether
// it can be rendered into an ASCII character. If not, use an escape.
for (let i = 0; i < value.length; i++) {
const char = value.substring(i, i + 1);
const charCode = char.charCodeAt(0);

if (charCode >= 20 && charCode <= 126) {
if (charCode === 34) {
bytesString += '\\' + char;
} else {
bytesString += char;
}
} else {
bytesString += `\\x${((charCode >> 4) & 0xf).toString(16)}${(charCode & 0xf).toString(16)}`;
}
}

return `b"${bytesString}"`;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import * as AnalyzerNodeInfo from '../analyzer/analyzerNodeInfo';
import { AliasDeclaration, Declaration, DeclarationType, isAliasDeclaration } from '../analyzer/declaration';
import {
areDeclarationsSame,
createSynthesizedAliasDeclaration,
getDeclarationsWithUsesLocalNameRemoved,
synthesizeAliasDeclaration,
} from '../analyzer/declarationUtils';
import { getModuleNode, getStringNodeValueRange } from '../analyzer/parseTreeUtils';
import { ParseTreeWalker } from '../analyzer/parseTreeWalker';
Expand Down Expand Up @@ -450,7 +450,7 @@ function _getDeclarationsForNonModuleNameNode(
const type = evaluator.getType(node);
if (type?.category === TypeCategory.Module) {
// Synthesize decl for the module.
return [createSynthesizedAliasDeclaration(type.priv.fileUri)];
return [synthesizeAliasDeclaration(type.priv.fileUri)];
}
}

Expand Down

0 comments on commit 020dea1

Please sign in to comment.