Skip to content

Isolated declarations errors #58201

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

Merged
merged 27 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
96de93c
Added command line option and errors
dragomirtitian Apr 15, 2024
94e1f5b
Added isolated declaration errors.
dragomirtitian Apr 15, 2024
5ed6046
Addressed code review.
dragomirtitian Apr 16, 2024
2160065
Improved expando function handling.
dragomirtitian Apr 16, 2024
461669d
Change how isolated declaration checks are called.
dragomirtitian Apr 17, 2024
987a476
Improve error for variables initialized with class expression
dragomirtitian Apr 17, 2024
5cb9fad
Forbid allowJS with isolated declarations and changed description of …
dragomirtitian Apr 18, 2024
652d1dc
Merge remote-tracking branch 'remotes/origin/main' into isolated-decl…
dragomirtitian Apr 18, 2024
186a8b8
Remove visitor cache workaround.
dragomirtitian Apr 18, 2024
d5a298d
Removed restriction on isolatedDeclaration running with out and outFile.
dragomirtitian Apr 18, 2024
75a43c4
Removed errors for enum members where there is no tantalizer emitted …
dragomirtitian Apr 18, 2024
5d747bb
Moved isolated declaration diagnostics to diagnostics.ts
dragomirtitian Apr 18, 2024
a831ae4
Changed diagnostic message texts.
dragomirtitian Apr 18, 2024
0b83d8a
Changed import to use ts namespace instead of the file.
dragomirtitian Apr 18, 2024
7ac2788
Renamed expressionOrTypeToTypeNode
dragomirtitian Apr 18, 2024
7c893ca
Fixed typo in diagnostic.
dragomirtitian Apr 19, 2024
b0ad92c
Forbid isolated declarations without declaration option.
dragomirtitian Apr 19, 2024
108fe87
Call expressionOrTypeToTypeNodeHelper instead expressionOrTypeToTypeNode
dragomirtitian Apr 19, 2024
27e1b8a
Removed drive by fix to shouldPrintWithInitializer
dragomirtitian Apr 19, 2024
53cb0ac
Use getEmitDeclarations instead of declaration
dragomirtitian Apr 19, 2024
5f3721c
Update src/compiler/program.ts
dragomirtitian Apr 19, 2024
bdd411a
Move methods off of context, fix context bug
jakebailey Apr 19, 2024
b761b3f
Merge pull request #144 from jakebailey/bloomberg-isolated-declaratio…
dragomirtitian Apr 19, 2024
4c9318c
Fix unused code lints
jakebailey Apr 19, 2024
102ebf7
Merge pull request #145 from jakebailey/bloomberg-isolated-declaratio…
dragomirtitian Apr 19, 2024
9a073be
Make isEntityNameVisible have the same parameter order
jakebailey Apr 19, 2024
ee7a872
Merge pull request #146 from jakebailey/bloomberg-isolated-declaratio…
dragomirtitian Apr 19, 2024
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
1 change: 1 addition & 0 deletions src/compiler/_namespaces/ts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export * from "../watchPublic";
export * from "../tsbuild";
export * from "../tsbuildPublic";
export * from "../executeCommandLine";
export * from "../expressionToTypeNode";
import * as moduleSpecifiers from "./ts.moduleSpecifiers";
export { moduleSpecifiers };
import * as performance from "./ts.performance";
Expand Down
92 changes: 85 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ import {
createPrinterWithRemoveCommentsOmitTrailingSemicolon,
createPropertyNameNodeForIdentifierOrLiteral,
createSymbolTable,
createSyntacticTypeNodeBuilder,
createTextWriter,
Debug,
Declaration,
Expand Down Expand Up @@ -387,6 +388,7 @@ import {
hasExtension,
HasIllegalDecorators,
HasIllegalModifiers,
hasInferredType,
HasInitializer,
hasInitializer,
hasJSDocNodes,
Expand Down Expand Up @@ -1481,6 +1483,17 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
var checkBinaryExpression = createCheckBinaryExpression();
var emitResolver = createResolver();
var nodeBuilder = createNodeBuilder();
var syntacticNodeBuilder = createSyntacticTypeNodeBuilder(compilerOptions, {
isEntityNameVisible,
isExpandoFunctionDeclaration,
isNonNarrowedBindableName,
getAllAccessorDeclarations: getAllAccessorDeclarationsForDeclaration,
requiresAddingImplicitUndefined,
isUndefinedIdentifierExpression(node: Identifier) {
Debug.assert(isExpressionNode(node));
return getSymbolAtLocation(node) === undefinedSymbol;
},
});
var evaluate = createEvaluator({
evaluateElementAccessExpression,
evaluateEntityNameExpression,
Expand Down Expand Up @@ -5864,7 +5877,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return meaning;
}

function isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult {
function isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node, shouldComputeAliasToMakeVisible = true): SymbolVisibilityResult {
const meaning = getMeaningOfEntityNameReference(entityName);
const firstIdentifier = getFirstIdentifier(entityName);
const symbol = resolveName(enclosingDeclaration, firstIdentifier.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*isUse*/ false);
Expand All @@ -5876,7 +5889,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

// Verify if the symbol is accessible
return (symbol && hasVisibleDeclarations(symbol, /*shouldComputeAliasToMakeVisible*/ true)) || {
return (symbol && hasVisibleDeclarations(symbol, shouldComputeAliasToMakeVisible)) || {
accessibility: SymbolAccessibility.NotAccessible,
errorSymbolName: getTextOfNode(firstIdentifier),
errorNode: firstIdentifier,
Expand Down Expand Up @@ -6018,7 +6031,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return setTextRangeWorker(setOriginalNode(range, location), location);
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const result = expressionOrTypeToTypeNode(context, expr, type, addUndefined);
const result = expressionOrTypeToTypeNodeHelper(context, expr, type, addUndefined);

(the function referenced here should be renamed) and the containing function should be renamed to expressionOrTypeToTypeNode. As-is, this is still only being invoked at that withContext callsite (which should also be expressionOrTypeToTypeNode once this is renamed), so that NoSyntacticPrinter flag isn't even doing anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

expressionOrTypeToTypeNodeHelper (after rename) may call typeToTypeNodeHelper which in turn will call serializeTypeForDeclaration and serializeReturnTypeForSignature and these would perform the check again. I can refactor those into serializeTypeForDeclarationHelper and serializeReturnTypeForSignatureHelper which do not perform the check and serializeTypeForDeclaration and serializeReturnTypeForSignature which would only be called from the top level. This would ensure the code does not check multiple times the same expression, and would remove the need for the NoSyntacticPrinter flag.

The flag will disappear in the next PRs. It's definitely not something that will remain. When the syntactic builder actually builds the types and falls back to the checked builder when it can't the problem of duplicate checks will go away.

Should I do the serializeTypeForDeclarationHelper and serializeReturnTypeForSignatureHelper refactor for now? Or ca we live with this until after the beta?

* Same as expressionOrTypeToTypeNodeHelper, but also checks if the expression can be syntactically typed.
*/
function expressionOrTypeToTypeNode(context: NodeBuilderContext, expr: Expression | JsxAttributeValue | undefined, type: Type, addUndefined?: boolean) {
const oldFlags = context.flags;
if (expr && !(context.flags & NodeBuilderFlags.NoSyntacticPrinter)) {
syntacticNodeBuilder.serializeTypeOfExpression(expr, context, addUndefined);
}
context.flags |= NodeBuilderFlags.NoSyntacticPrinter;
const result = expressionOrTypeToTypeNodeHelper(context, expr, type, addUndefined);
context.flags = oldFlags;
return result;
}
function expressionOrTypeToTypeNodeHelper(context: NodeBuilderContext, expr: Expression | JsxAttributeValue | undefined, type: Type, addUndefined?: boolean) {
if (expr) {
const typeNode = isAssertionExpression(expr) ? expr.type
: isJSDocTypeAssertion(expr) ? getJSDocTypeAssertionType(expr)
Expand Down Expand Up @@ -8155,6 +8181,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
const decl = declaration ?? symbol.valueDeclaration ?? symbol.declarations?.[0];
const expr = decl && isDeclarationWithPossibleInnerTypeNodeReuse(decl) ? getPossibleTypeNodeReuseExpression(decl) : undefined;

if (decl && hasInferredType(decl) && !(context.flags & NodeBuilderFlags.NoSyntacticPrinter)) {
syntacticNodeBuilder.serializeTypeOfDeclaration(decl, context);
}
context.flags |= NodeBuilderFlags.NoSyntacticPrinter;
const result = expressionOrTypeToTypeNode(context, expr, type, addUndefined);
context.flags = oldFlags;
return result;
Expand All @@ -8177,6 +8207,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
let returnTypeNode: TypeNode | undefined;
const returnType = getReturnTypeOfSignature(signature);
if (returnType && !(suppressAny && isTypeAny(returnType))) {
if (signature.declaration && !(context.flags & NodeBuilderFlags.NoSyntacticPrinter)) {
syntacticNodeBuilder.serializeReturnTypeForSignature(signature.declaration, context);
}
context.flags |= NodeBuilderFlags.NoSyntacticPrinter;
returnTypeNode = serializeReturnTypeForSignatureWorker(context, signature);
}
else if (!suppressAny) {
Expand Down Expand Up @@ -45617,9 +45651,10 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
result.value,
/*isSyntacticallyString*/ false,
/*resolvedOtherFiles*/ true,
/*hasExternalReferences*/ true,
);
}
return result;
return evaluatorResult(result.value, result.isSyntacticallyString, result.resolvedOtherFiles, /*hasExternalReferences*/ true);
}
}
return evaluatorResult(/*value*/ undefined);
Expand Down Expand Up @@ -45651,7 +45686,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
error(expr, Diagnostics.A_member_initializer_in_a_enum_declaration_cannot_reference_members_declared_after_it_including_members_defined_in_other_enums);
return evaluatorResult(/*value*/ 0);
}
return getEnumMemberValue(declaration as EnumMember);
const value = getEnumMemberValue(declaration as EnumMember);
if (location.parent !== declaration.parent) {
return evaluatorResult(value.value, value.isSyntacticallyString, value.resolvedOtherFiles, /*hasExternalReferences*/ true);
}
return value;
}

function checkEnumDeclaration(node: EnumDeclaration) {
Expand Down Expand Up @@ -48267,12 +48306,25 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function isExpandoFunctionDeclaration(node: Declaration): boolean {
const declaration = getParseTreeNode(node, isFunctionDeclaration);
const declaration = getParseTreeNode(node, (n): n is FunctionDeclaration | VariableDeclaration => isFunctionDeclaration(n) || isVariableDeclaration(n));
if (!declaration) {
return false;
}
const symbol = getSymbolOfDeclaration(declaration);
if (!symbol || !(symbol.flags & SymbolFlags.Function)) {
let symbol: Symbol | undefined;
if (isVariableDeclaration(declaration)) {
if (declaration.type || (!isInJSFile(declaration) && !isVarConstLike(declaration))) {
return false;
}
const initializer = getDeclaredExpandoInitializer(declaration);
if (!initializer || !canHaveSymbol(initializer)) {
return false;
}
symbol = getSymbolOfDeclaration(initializer);
}
else {
symbol = getSymbolOfDeclaration(declaration);
}
if (!symbol || !(symbol.flags & SymbolFlags.Function | SymbolFlags.Variable)) {
return false;
}
return !!forEachEntry(getExportsOfSymbol(symbol), p => p.flags & SymbolFlags.Value && isExpandoPropertyDeclaration(p.valueDeclaration));
Expand Down Expand Up @@ -48609,6 +48661,25 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}
return false;
}
function isNonNarrowedBindableName(node: ComputedPropertyName) {
if (!hasBindableName(node.parent)) {
return false;
}

const expression = node.expression;
if (!isEntityNameExpression(expression)) {
return true;
}

const type = getTypeOfExpression(expression);
const symbol = getSymbolAtLocation(expression);
if (!symbol) {
return false;
}
// Ensure not type narrowing
const declaredType = getTypeOfSymbol(symbol);
return declaredType === type;
}

function literalTypeToNode(type: FreshableType, enclosing: Node, tracker: SymbolTracker): Expression {
const enumResult = type.flags & TypeFlags.EnumLike ? nodeBuilder.symbolToExpression(type.symbol, SymbolFlags.Value, enclosing, /*flags*/ undefined, tracker)
Expand Down Expand Up @@ -48734,6 +48805,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return node && getExternalModuleFileFromDeclaration(node);
},
isLiteralConstDeclaration,
isNonNarrowedBindableName,
isLateBound: (nodeIn: Declaration): nodeIn is LateBoundDeclaration => {
const node = getParseTreeNode(nodeIn, isDeclaration);
const symbol = node && getSymbolOfDeclaration(node);
Expand Down Expand Up @@ -51170,4 +51242,10 @@ class SymbolTrackerImpl implements SymbolTracker {
private onDiagnosticReported() {
this.context.reportedDiagnostic = true;
}

reportInferenceFallback(node: Node): void {
if (this.inner?.reportInferenceFallback) {
this.inner.reportInferenceFallback(node);
}
}
}
9 changes: 9 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,6 +830,15 @@ const commandOptionsWithoutBuild: CommandLineOption[] = [
description: Diagnostics.Do_not_transform_or_elide_any_imports_or_exports_not_marked_as_type_only_ensuring_they_are_written_in_the_output_file_s_format_based_on_the_module_setting,
defaultValueDescription: false,
},
{
name: "isolatedDeclarations",
type: "boolean",
category: Diagnostics.Interop_Constraints,
description: Diagnostics.Require_sufficient_annotation_on_exports_so_other_tools_can_trivially_generate_declaration_files,
defaultValueDescription: false,
affectsBuildInfo: true,
affectsSemanticDiagnostics: true,
},

// Strict Type Checks
{
Expand Down
125 changes: 125 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -6227,6 +6227,11 @@
"category": "Message",
"code": 6718
},
"Require sufficient annotation on exports so other tools can trivially generate declaration files.": {
"category": "Message",
"code": 6719
},

"Default catch clause variables as 'unknown' instead of 'any'.": {
"category": "Message",
"code": 6803
Expand Down Expand Up @@ -6741,6 +6746,126 @@
"category": "Error",
"code": 9006
},
"Function must have an explicit return type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9007
},
"Method must have an explicit return type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9008
},
"At least one accessor must have an explicit return type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9009
},
"Variable must have an explicit type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9010
},
"Parameter must have an explicit type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9011
},
"Property must have an explicit type annotation with --isolatedDeclarations.": {
"category": "Error",
"code": 9012
},
"Expression type can't be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9013
},
"Computed properties must be number or string literals, variables or dotted expressions with --isolatedDeclarations.": {
"category": "Error",
"code": 9014
},
"Objects that contain spread assignments can't be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9015
},
"Objects that contain shorthand properties can't be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9016
},
"Only const arrays can be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9017
},
"Arrays with spread elements can't inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9018
},
"Binding elements can't be exported directly with --isolatedDeclarations.": {
"category": "Error",
"code": 9019
},
"Enum member initializers must be computable without references to external symbols with --isolatedDeclarations.": {
"category": "Error",
"code": 9020
},
"Extends clause can't contain an expression with --isolatedDeclarations.": {
"category": "Error",
"code": 9021
},
"Inference from class expressions is not supported with --isolatedDeclarations.": {
"category": "Error",
"code": 9022
},
"Assigning properties to functions without declaring them is not supported with --isolatedDeclarations. Add an explicit declaration for the properties assigned to this function.": {
"category": "Error",
"code": 9023
},
"Declaration emit for this parameter requires implicitly adding undefined to it's type. This is not supported with --isolatedDeclarations.": {
"category": "Error",
"code": 9025
},
"Declaration emit for this file requires preserving this import for augmentations. This is not supported with --isolatedDeclarations.": {
"category": "Error",
"code": 9026
},
"Add a type annotation to the variable {0}.": {
"category": "Error",
"code": 9027
},
"Add a type annotation to the parameter {0}.": {
"category": "Error",
"code": 9028
},
"Add a type annotation to the property {0}.": {
"category": "Error",
"code": 9029
},
"Add a return type to the function expression.": {
"category": "Error",
"code": 9030
},
"Add a return type to the function declaration.": {
"category": "Error",
"code": 9031
},
"Add a return type to the get accessor declaration.": {
"category": "Error",
"code": 9032
},
"Add a type to parameter of the set accessor declaration.": {
"category": "Error",
"code": 9033
},
"Add a return type to the method": {
"category": "Error",
"code": 9034
},
"Add satisfies and a type assertion to this expression (satisfies T as T) to make the type explicit.": {
"category": "Error",
"code": 9035
},
"Move the expression in default export to a variable and add a type annotation to it.": {
"category": "Error",
"code": 9036
},
"Default exports can't be inferred with --isolatedDeclarations.": {
"category": "Error",
"code": 9037
},
"JSX attributes must only be assigned a non-empty 'expression'.": {
"category": "Error",
"code": 17000
Expand Down
1 change: 1 addition & 0 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,7 @@ export const notImplementedResolver: EmitResolver = {
isArgumentsLocalBinding: notImplemented,
getExternalModuleFileFromDeclaration: notImplemented,
isLiteralConstDeclaration: notImplemented,
isNonNarrowedBindableName: notImplemented,
getJsxFactoryEntity: notImplemented,
getJsxFragmentFactoryEntity: notImplemented,
isBindingCapturedByNode: notImplemented,
Expand Down
Loading