Integrated fixed test. #106

dragomirtitian · 2023-11-13T15:10:00Z

No description provided.

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

h-joo · 2023-11-14T00:35:17Z

.../baselines/reference/isolated-declarations/auto-fixed/diff/commonMissingSemicolons.d.ts.diff

+ !!! error TS1005: ';' expected.
+     function function(): void { }
+            
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


This is a weird error, I guess we can detect this case and not print errors, on it WDYT?

I don't think there is a good way to do this unless we are explicitly redetecting the syntax errors which is not something I would actually want to do. I will make a note to ask MS as well

h-joo · 2023-11-14T00:39:55Z

...e/isolated-declarations/auto-fixed/diff/collisionCodeGenEnumWithEnumMemberConflict.d.ts.diff

+        Color,
+        Thing = Color
+        ~~~~~
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


This looks wrong. Color should get assigned 0 by the constEvaluate and thus Thing should also be zero if I try to recall the logic.

h-joo · 2023-11-14T00:41:10Z

.../reference/isolated-declarations/auto-fixed/diff/classMemberWithMissingIdentifier2.d.ts.diff

+
+ !!! error TS1146: Declaration expected.
+              
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


We seem to have a general problem on reporting around ill-formed syntax. What do you think is the right way to go?

h-joo · 2023-11-14T00:58:45Z

src/compiler/transformers/declarations/emit-resolver.ts

+        if (isStringLiteralLike(node)) {
+            return factory.createStringLiteral(node.text);
+        }
+


Nit: would it be better to make line breaks more consistent?

h-joo · 2023-11-14T01:28:00Z

src/compiler/transformers/declarations/transform-project.ts

-    options.declaration = true;
-    options.declarationMap = true;
-    const emitHost = createEmitDeclarationHost(options, sys);
+export function transpileDeclaration(sourceFile: SourceFile, emitHost: IsolatedEmitHost) {


Please mark this @internal. Every file reading in the typescript.d.ts in will fail due to IsolatedEmitHost being internal.

We should discuss this. Is this a fully internal API? I would rather mark IsolatedEmitHost as being public.

I guess that's fine for me.

h-joo · 2023-11-14T04:09:23Z

...e/isolated-declarations/auto-fixed/diff/expandoFunctionExpressionsWithDynamicNames.d.ts.diff

+    
+    export const expr = (): void => {}
+                 ~~~~
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


This shouldn't have triggered as it does have type annotation on the RHS.

empty parameter.

void as a return type.
Triggering TS9009 should be enough.

Agreed. Made a node to fix in a follow on PER

...jsxNamespaceImplicitImportJSXNamespaceFromConfigPickedOverGlobalOne(jsx=react-jsx).d.ts.diff

h-joo · 2023-11-14T04:34:21Z

...nce/isolated-declarations/auto-fixed/diff/noImplicitAnyDestructuringVarDeclaration.d.ts.diff

+ !!! error TS1182: A destructuring declaration must have an initializer.
+          ~
+ !!! error TS7031: Binding element 'a' implicitly has an 'any' type.
+         ~


This is its own error - we should not report.

We don't really do a lot of correlation between errors.

h-joo · 2023-11-14T04:37:04Z

tests/baselines/reference/isolated-declarations/auto-fixed/diff/mergedDeclarations2.d.ts.diff

+     enum Foo {
+         a = b
+        ~
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


oh, we seems not to support merged enum value assignments?

No we don't they can merge across files which is something we definitely don't want.

h-joo · 2023-11-14T04:39:08Z

...es/reference/isolated-declarations/auto-fixed/diff/negateOperatorInvalidOperations.d.ts.diff

+     // operand before -
+     var NUMBER1 = var NUMBER: any-;  //expect error
+                 
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


invalid syntax.

Yeah. This test is too invalid. I did not review all of them yet.

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

dragomirtitian · 2023-11-13T15:14:45Z

src/compiler/transformers/declarations/transform-project.ts

-    if (diagnostics.length > 0) {
-        throw new Error(`Cannot transform file '${sourceFile.fileName}' due to ${diagnostics.length} diagnostics`);
-    }


I removed the throw here. The compiler does not generally throw errors on invalid code.

dragomirtitian · 2023-11-14T09:53:43Z

tests/baselines/reference/isolated-declarations/auto-fixed/diff/mergedDeclarations2.d.ts.diff

+     enum Foo {
+         a = b
+        ~
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


No we don't they can merge across files which is something we definitely don't want.

dragomirtitian · 2023-11-14T09:54:11Z

...es/reference/isolated-declarations/auto-fixed/diff/negateOperatorInvalidOperations.d.ts.diff

+     // operand before -
+     var NUMBER1 = var NUMBER: any-;  //expect error
+                 
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


Yeah. This test is too invalid. I did not review all of them yet.

dragomirtitian · 2023-11-14T09:56:30Z

...nce/isolated-declarations/auto-fixed/diff/noImplicitAnyDestructuringVarDeclaration.d.ts.diff

+ !!! error TS1182: A destructuring declaration must have an initializer.
+          ~
+ !!! error TS7031: Binding element 'a' implicitly has an 'any' type.
+         ~


We don't really do a lot of correlation between errors.

dragomirtitian · 2023-11-14T09:58:49Z

...e/isolated-declarations/auto-fixed/diff/expandoFunctionExpressionsWithDynamicNames.d.ts.diff

+    
+    export const expr = (): void => {}
+                 ~~~~
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


Agreed. Made a node to fix in a follow on PER

dragomirtitian · 2023-11-14T09:59:40Z

src/compiler/transformers/declarations/transform-project.ts

-    options.declaration = true;
-    options.declarationMap = true;
-    const emitHost = createEmitDeclarationHost(options, sys);
+export function transpileDeclaration(sourceFile: SourceFile, emitHost: IsolatedEmitHost) {


We should discuss this. Is this a fully internal API? I would rather mark IsolatedEmitHost as being public.

dragomirtitian · 2023-11-14T10:03:25Z

.../baselines/reference/isolated-declarations/auto-fixed/diff/commonMissingSemicolons.d.ts.diff

+ !!! error TS1005: ';' expected.
+     function function(): void { }
+            
+!!! error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.


I don't think there is a good way to do this unless we are explicitly redetecting the syntax errors which is not something I would actually want to do. I will make a note to ask MS as well

dragomirtitian · 2023-11-14T10:19:22Z

...ated-declarations/auto-fixed/diff/declarationEmitDefaultExportWithStaticAssignment.d.ts.diff

+    export {Foo};
+    export default function Example(): void {}
+                            ~~~~~~~
+!!! error TS9009: Assigning properties to functions without declaring them is not supported with --isolatedDeclarations. Add an explicit declaration for the properties assigned to this function.


Not on this PR, but we should look at improving this. Noted

dragomirtitian · 2023-11-14T10:20:25Z

tests/baselines/reference/isolated-declarations/original/tsc/constEnums.d.ts

 constEnums.ts(33,5): error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.
+constEnums.ts(33,10): error TS2474: const enum member initializers must be constant expressions.


Yeah. I was working on this one.

dragomirtitian · 2023-11-14T10:21:16Z

tests/baselines/reference/isolated-declarations/original/diff/constEnums.d.ts.diff

+             export module C {
+                 export const enum E {
+                     V1 = 1,
+                     V2 = A.B.C.E.V1 | 100


yes. we should porobably support it.

dragomirtitian added 9 commits November 13, 2023 15:05

Integrated fixed tests.

ae61dd6

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Added test baseline

d6f9d2d

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Fixed most tests after merging features.

9c80f76

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Remove path from test.

ea7b82a

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Added new line before errors.

36e5505

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Disable forceDtsEmit for fixed tests.

4a4312f

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Fixed source maps differences.

5a7784c

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Removed comment

9856f97

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

Added missing baseline.

3936d4a

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

dragomirtitian force-pushed the isolated-declarations-tests branch from ec46804 to 3936d4a Compare November 13, 2023 17:07

h-joo approved these changes Nov 14, 2023

View reviewed changes

Removed unfixed tests.

ffe4b1c

Signed-off-by: Titian Cernicova-Dragomir <tcernicovad1@bloomberg.net>

dragomirtitian commented Nov 14, 2023

View reviewed changes

dragomirtitian merged commit 3023b9f into isolated-declarations Nov 14, 2023
1 check passed

dragomirtitian deleted the isolated-declarations-tests branch November 17, 2023 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integrated fixed test. #106

Integrated fixed test. #106

dragomirtitian commented Nov 13, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

h-joo Nov 14, 2023

h-joo Nov 14, 2023

h-joo Nov 14, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

h-joo Nov 14, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

h-joo Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 13, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

dragomirtitian Nov 14, 2023

		constEnums.ts(33,5): error TS9007: Declaration emit for this file requires type resolution. An explicit type annotation may unblock declaration emit.
		constEnums.ts(33,10): error TS2474: const enum member initializers must be constant expressions.

Integrated fixed test. #106

Integrated fixed test. #106

Conversation

dragomirtitian commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment