919 Commits

Author SHA1 Message Date
Pete Bacon Darwin
a8aa2caeb6 refactor(compiler-cli): support Buffer files in FileSystem (#36843)
Adding `readFileBuffer()` method and allowing `writeFile()` to accept a
Buffer object will be useful when reading and writing non-text files,
such as is done in the `@angular/localize` package.

PR Close #36843
2020-05-13 15:52:49 -07:00
Ayaz Hafiz
ba1dfd0cf3 feat(compiler): add name spans for property reads and method calls (#36826)
ASTs for property read and method calls contain information about
the entire span of the expression, including its receiver. Use cases
like a language service and compile error messages may be more
interested in the span of the direct identifier for which the
expression is constructed (i.e. an accessed property). To support this,
this commit adds a `nameSpan` property on

- `PropertyRead`s
- `SafePropertyRead`s
- `PropertyWrite`s
- `MethodCall`s
- `SafeMethodCall`s

The `nameSpan` property already existed for `BindingPipe`s.

This commit also updates usages of these expressions' `sourceSpan`s in
Ngtsc and the langauge service to use `nameSpan`s where appropriate.

PR Close #36826
2020-05-08 14:42:43 -07:00
Pete Bacon Darwin
99b63b677e Revert "fix(compiler-cli): fix case-sensitivity issues in NgtscCompilerHost (#36968)" (#37003)
This reverts commit 4abd60361a7f27305c5add4f14927caf63482af3.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
00b9de56f5 Revert "fix(compiler-cli): ensure LogicalFileSystem handles case-sensitivity (#36968)" (#37003)
This reverts commit 65337fb8b8d9d16407c182af935dd737beaea7dd.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
4423ab5109 Revert "fix(compiler-cli): ensure getRootDirs() handles case-sensitivity (#36968)" (#37003)
This reverts commit 5bddeea559754283d6f51741741a313f8f4c20be.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
0fc0d9bb3d Revert "fix(compiler-cli): isCaseSensitive() returns correct value (#36968)" (#37003)
This reverts commit 4becc1bc9531b79e0d834e453981655897571bdf.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
00cc02fb0c Revert "fix(compiler-cli): ensure MockFileSystem handles case-sensitivity (#36968)" (#37003)
This reverts commit b6c042d0a35b977fc1ab51827805b73dfceefabb.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
0783d482a7 Revert "fix(compiler-cli): normalize mock Windows file paths correctly (#36968)" (#37003)
This reverts commit 654868f5840597b16adf9ff7f71d0e9f32f04946.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
6cce05e2c5 Revert "fix(compiler-cli): use CompilerHost to ensure canonical file paths (#36968)" (#37003)
This reverts commit 7e9d5f5e826ee80b60d6f6e6e5d17dc94cb491c3.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
bf446cb2d5 Revert "test(compiler-cli): ensure indexer tests are not brittle to case-sensitivity (#36968)" (#37003)
This reverts commit 3361f59a4f26c181f22dfe6181592f82ec115798.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
04fd90e32a Revert "test(compiler-cli): ensure partial-evaluator tests are not brittle to case-sensitivity (#36968)" (#37003)
This reverts commit c6e5225ec346b894d1890cb1e320dca7deeb6615.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
be06a44861 Revert "test(compiler-cli): ensure reflection tests are not brittle to case-sensitivity (#36968)" (#37003)
This reverts commit dcf1dcb757d523bb2347fb7dfecb5985a1106ed7.

The changes to the case-sensitivity handling in #36968 caused multiple
projects to fail to build. See #36992, #36993 and #37000.

The issue is related to the logical path handling. But it is felt that
it is safer to revert the entire PR and then to investigate further.

PR Close #37003
2020-05-08 09:48:19 -07:00
Pete Bacon Darwin
dcf1dcb757 test(compiler-cli): ensure reflection tests are not brittle to case-sensitivity (#36968)
These tests were matching file-paths against what is retrieved from the
TS compiler. But the TS compiler paths have been canonicalised, so the
tests were brittle on case-insensitive file-systems.

PR Close #36968
2020-05-07 10:32:18 -07:00
Pete Bacon Darwin
c6e5225ec3 test(compiler-cli): ensure partial-evaluator tests are not brittle to case-sensitivity (#36968)
These tests were matching file-paths against what is retrieved from the
TS compiler. But the TS compiler paths have been canonicalised, so the
tests were brittle on case-insensitive file-systems.

PR Close #36968
2020-05-07 10:32:18 -07:00
Pete Bacon Darwin
3361f59a4f test(compiler-cli): ensure indexer tests are not brittle to case-sensitivity (#36968)
These tests were matching file-paths against what is retrieved from the
TS compiler. But the TS compiler paths have been canonicalised, so the
tests were brittle on case-insensitive file-systems.

PR Close #36968
2020-05-07 10:32:18 -07:00
Pete Bacon Darwin
7e9d5f5e82 fix(compiler-cli): use CompilerHost to ensure canonical file paths (#36968)
The type checking infrastrure uses file-paths that may come from the
TS compiler. Such paths will have been canonicalized, and so the type
checking classes must also canonicalize paths when matching.

PR Close #36968
2020-05-07 10:32:18 -07:00
Pete Bacon Darwin
654868f584 fix(compiler-cli): normalize mock Windows file paths correctly (#36968)
Since the `MockFileSystemWindows` is case-insensitive, any
drive path that must be added to a normalized path should be lower
case to make the path canonical.

PR Close #36968
2020-05-07 10:32:18 -07:00
Pete Bacon Darwin
b6c042d0a3 fix(compiler-cli): ensure MockFileSystem handles case-sensitivity (#36968)
Previously this class used the file passed in directly to look up files in the
in-memory mock file-system. But this doesn't match the behaviour of
case-insensitive file-systems. Now the look up is done on the canonical
file paths.

PR Close #36968
2020-05-07 10:32:17 -07:00
Pete Bacon Darwin
4becc1bc95 fix(compiler-cli): isCaseSensitive() returns correct value (#36968)
Previously this method was returning the exact opposite value
than the correct one.
Also, calling `this.exists()` causes an infinite recursions,
so the actual file-system `fs.existsSync()` method is used
to ascertain the case-sensitivity of the file-system.

PR Close #36968
2020-05-07 10:32:17 -07:00
Pete Bacon Darwin
5bddeea559 fix(compiler-cli): ensure getRootDirs() handles case-sensitivity (#36968)
Previously the `getRootDirs()` function was not converting
the root directory paths to their canonical form, which can
cause problems on case-insensitive file-systems.

PR Close #36968
2020-05-07 10:32:17 -07:00
Pete Bacon Darwin
65337fb8b8 fix(compiler-cli): ensure LogicalFileSystem handles case-sensitivity (#36968)
The `LogicalFileSystem` was not taking into account the
case-sensitivity of the file-system when caching logical
file paths.

PR Close #36968
2020-05-07 10:32:17 -07:00
Pete Bacon Darwin
4abd60361a fix(compiler-cli): fix case-sensitivity issues in NgtscCompilerHost (#36968)
The `getCanonicalFileName()` method was not actually
calling the  `useCaseSensitiveFileNames()` method. So
it always returned a case-sensitive canonical filename.

PR Close #36968
2020-05-07 10:32:17 -07:00
JoostK
c440165384 fix(ngcc): recognize enum declarations emitted in JavaScript (#36550)
An enum declaration in TypeScript code will be emitted into JavaScript
as a regular variable declaration, with the enum members being declared
inside an IIFE. For ngcc to support interpreting such variable
declarations as enum declarations with its members, ngcc needs to
recognize the enum declaration emit structure and extract all member
from the statements in the IIFE.

This commit extends the `ConcreteDeclaration` structure in the
`ReflectionHost` abstraction to be able to capture the enum members
on a variable declaration, as a substitute for the original
`ts.EnumDeclaration` as it existed in TypeScript code. The static
interpreter has been extended to handle the extracted enum members
as it would have done for `ts.EnumDeclaration`.

Fixes #35584
Resolves FW-2069

PR Close #36550
2020-04-28 15:59:58 -07:00
Pete Bacon Darwin
95f45e8070 refactor(compiler): remove unused CachedFileSystem (#36687)
This was only being used by ngcc but not any longer.

PR Close #36687
2020-04-17 16:33:49 -04:00
JoostK
468cf69c55 fix(compiler): handle type references to namespaced symbols correctly (#36106)
When the compiler needs to convert a type reference to a value
expression, it may encounter a type that refers to a namespaced symbol.
Such namespaces need to be handled specially as there's various forms
available. Consider a namespace named "ns":

1. One can refer to a namespace by itself: `ns`. A namespace is only
   allowed to be used in a type position if it has been merged with a
   class, but even if this is the case it may not be possible to convert
   that type into a value expression depending on the import form. More
   on this later (case a below)
2. One can refer to a type within the namespace: `ns.Foo`. An import
   needs to be generated to `ns`, from which the `Foo` property can then
   be read.
3. One can refer to a type in a nested namespace within `ns`:
   `ns.Foo.Bar` and possibly even deeper nested. The value
   representation is similar to case 2, but includes additional property
   accesses.

The exact strategy of how to deal with these cases depends on the type
of import used. There's two flavors available:

a. A namespaced import like `import * as ns from 'ns';` that creates
   a local namespace that is irrelevant to the import that needs to be
   generated (as said import would be used instead of the original
   import).

   If the local namespace "ns" itself is referred to in a type position,
   it is invalid to convert it into a value expression. Some JavaScript
   libraries publish a value as default export using `export = MyClass;`
   syntax, however it is illegal to refer to that value using "ns".
   Consequently, such usage in a type position *must* be accompanied by
   an `@Inject` decorator to provide an explicit token.

b. An explicit namespace declaration within a module, that can be
   imported using a named import like `import {ns} from 'ns';` where the
   "ns" module declares a namespace using `declare namespace ns {}`.
   In this case, it's the namespace itself that needs to be imported,
   after which any qualified references into the namespace are converted
   into property accesses.

Before this change, support for namespaces in the type-to-value
conversion was limited and only worked  correctly for a single qualified
name using a namespace import (case 2a). All other cases were either
producing incorrect code or would crash the compiler (case 1a).

Crashing the compiler is not desirable as it does not indicate where
the issue is. Moreover, the result of a type-to-value conversion is
irrelevant when an explicit injection token is provided using `@Inject`,
so referring to a namespace in a type position (case 1) could still be
valid.

This commit introduces logic to the type-to-value conversion to be able
to properly deal with all type references to namespaced symbols.

Fixes #36006
Resolves FW-1995

PR Close #36106
2020-04-09 11:32:22 -07:00
Alex Rickabaugh
d5aa6b5bd6 style(compiler-cli): reformat of codebase with new clang-format version (#36520)
This commit reformats the packages/compiler-cli tree using the new version
of clang-format.

PR Close #36520
2020-04-08 14:51:08 -07:00
JiaLiPassion
421b6a97d6 fix(zone.js): add issue numbers of @types/jasmine to the test cases (#34625)
Some cases will still need to use `spy as any` cast, because `@types/jasmine` have some issues,
1. The issue jasmine doesn't handle optional method properties, https://github.com/DefinitelyTyped/DefinitelyTyped/issues/43486
2. The issue jasmine doesn't handle overload method correctly, https://github.com/DefinitelyTyped/DefinitelyTyped/issues/42455

PR Close #34625
2020-04-08 12:10:35 -07:00
JiaLiPassion
b28a5f6eef build: update jasmine to 3.5 (#34625)
1. update jasmine to 3.5
2. update @types/jasmine to 3.5
3. update @types/jasminewd2 to 2.0.8

Also fix several cases, the new jasmine 3 will help to create test cases correctly,
such as in the `jasmine 2.x` version, the following case will pass

```
expect(1 == 2);
```

But in jsamine 3, the case will need to be

```
expect(1 == 2).toBeTrue();
```

PR Close #34625
2020-04-08 12:10:34 -07:00
JoostK
cbc25bbf31 fix(compiler): resolve enum values in binary operations (#36461)
During static evaluation of expressions, the partial evaluator
may come across a binary + operator for which it needs to
evaluate its operands. Any of these operands may be a reference
to an enum member, in which case the enum member's value needs
to be used as literal value, not the enum member reference
itself. This commit fixes the behavior by resolving an
`EnumValue` when used as a literal value.

Fixes #35584
Resolves FW-1951

PR Close #36461
2020-04-07 15:21:52 -07:00
JoostK
7385b2cf25 style(compiler): reformat partial evaluator source tree (#36461)
PR Close #36461
2020-04-07 15:21:52 -07:00
George Kalpakas
0daa48800e fix(ngcc): correctly identify relative Windows-style import paths (#36372)
Previously, `isRelativePath()` assumed paths are *nix-style. This caused
Windows-style paths (such as `C:\foo\some-package\some-file.js`) to not
be recognized as "relative" imports.

This commit fixes this by using the OS-agnostic `isRooted()` helper and
also accounting for both styles of path delimiters: `/` and `\`

PR Close #36372
2020-04-07 15:21:27 -07:00
Ayaz Hafiz
4894220acf fix(compiler-cli): pass real source spans where they are empty (#31805)
Some consumers of functions that take `ParseSourceSpan`s currently pass
empty and incorrect source spans. This fixes those cases.

PR Close #31805
2020-04-06 09:28:27 -07:00
JoostK
21da0346c7 refactor(compiler): add @nocollapse annotation using a synthetic comment (#35932)
In Ivy, Angular decorators are compiled into static fields that are
inserted into a class declaration in a TypeScript transform. When
targeting Closure compiler such fields need to be annotated with
`@nocollapse` to prevent them from being lifted from a static field into
a variable, as that would prevent the Ivy runtime from being able to
find the compiled definitions.

Previously, there was a bug in TypeScript where synthetic comments added
in a transform would not be emitted at all, so as a workaround a global
regex-replace was done in the emit's `writeFile` callback that would add
the `@nocollapse` annotation to all static Ivy definition fields. This
approach is no longer possible when ngtsc is running as TypeScript
plugin, as a plugin cannot control emit behavior.

The workaround is no longer necessary, as synthetic comments are now
properly emitted, likely as of
https://github.com/microsoft/TypeScript/pull/22141 which has been
released with TypeScript 2.8.

This change is required for running ngtsc as TypeScript plugin in
Bazel's `ts_library` rule, to move away from the custom `ngc_wrapped`
approach.

Resolves FW-1952

PR Close #35932
2020-04-01 15:37:06 -07:00
JoostK
fb70083339 feat(compiler): add dependency info and ng-content selectors to metadata (#35695)
This commit augments the `FactoryDef` declaration of Angular decorated
classes to contain information about the parameter decorators used in
the constructor. If no constructor is present, or none of the parameters
have any Angular decorators, then this will be represented using the
`null` type. Otherwise, a tuple type is used where the entry at index `i`
corresponds with parameter `i`. Each tuple entry can be one of two types:

1. If the associated parameter does not have any Angular decorators,
   the tuple entry will be the `null` type.
2. Otherwise, a type literal is used that may declare at least one of
   the following properties:
   - "attribute": if `@Attribute` is present. The injected attribute's
   name is used as string literal type, or the `unknown` type if the
   attribute name is not a string literal.
   - "self": if `@Self` is present, always of type `true`.
   - "skipSelf": if `@SkipSelf` is present, always of type `true`.
   - "host": if `@Host` is present, always of type `true`.
   - "optional": if `@Optional` is present, always of type `true`.

   A property is only present if the corresponding decorator is used.

   Note that the `@Inject` decorator is currently not included, as it's
   non-trivial to properly convert the token's value expression to a
   type that is valid in a declaration file.

Additionally, the `ComponentDefWithMeta` declaration that is created for
Angular components has been extended to include all selectors on
`ng-content` elements within the component's template.

This additional metadata is useful for tooling such as the Angular
Language Service, as it provides the ability to offer suggestions for
directives/components defined in libraries. At the moment, such
tooling extracts the necessary information from the _metadata.json_
manifest file as generated by ngc, however this metadata representation
is being replaced by the information emitted into the declaration files.

Resolves FW-1870

PR Close #35695
2020-03-24 14:21:42 -07:00
ayazhafiz
df890d7629 fix(compiler): record correct end of expression (#34690)
This commit fixes a bug with the expression parser wherein the end index
of an expression node was recorded as the start index of the next token,
not the end index of the current token.

Closes #33477
Closes https://github.com/angular/vscode-ng-language-service/issues/433

PR Close #34690
2020-03-20 10:19:02 -07:00
Alex Rickabaugh
e3ecdc6a63 feat(bazel): transform generated shims (in Ivy) with tsickle (#35975)
Currently, when Angular code is built with Bazel and with Ivy, generated
factory shims (.ngfactory files) are not processed via the majority of
tsickle's transforms. This is a subtle effect of the build infrastructure,
but it boils down to a TsickleHost method `shouldSkipTsickleProcessing`.

For ngc_wrapped builds (Bazel + Angular), this method is defined in the
`@bazel/typescript` (aka bazel rules_typescript) implementation of
`CompilerHost`. The default behavior is to skip tsickle processing for files
which are not present in the original `srcs[]` of the build rule. In
Angular's case, this includes all generated shim files.

For View Engine factories this is probably desirable as they're quite
complex and they've never been tested with tsickle. Ivy factories however
are smaller and very straightforward, and it makes sense to treat them like
any other output.

This commit adjusts two independent implementations of
`shouldSkipTsickleProcessing` to enable transformation of Ivy shims:

* in `@angular/bazel` aka ngc_wrapped, the upstream `@bazel/typescript`
  `CompilerHost` is patched to treat .ngfactory files the same as their
  original source file, with respect to tsickle processing.

  It is currently not possible to test this change as we don't have any test
  that inspects tsickle output with bazel. It will be extensively tested in
  g3.

* in `ngc`, Angular's own implementation is adjusted to allow for the
  processing of shims when compiling with Ivy. This enables a unit test to
  be written to validate the correct behavior of tsickle when given a host
  that's appropriately configured to process factory shims.

For ngtsc-as-a-plugin, a similar fix will need to be submitted upstream in
tsc_wrapped.

PR Close #35848

PR Close #35975
2020-03-17 10:17:28 -07:00
Alex Rickabaugh
95c729f5d1 build: typescript 3.8 support (#35864)
This commit adds support in the Angular monorepo and in the Angular
compiler(s) for TypeScript 3.8. All packages can now compile with
TS 3.8.

For most of the repo, only a handful few typings adjustments were needed:

* TS 3.8 has a new `CustomElementConstructor` DOM type, which enforces a
  zero-argument constructor. The `NgElementConstructor` type previously
  declared a required `injector` argument despite the fact that its
  implementation allowed `injector` to be optional. The interface type was
  updated to reflect the optionality of the argument.
* Certain error messages were changed, and expectations in tests were
  updated as a result.
* tsserver (part of language server) now returns performance information in
  responses, so test expectations were changed to only assert on the actual
  body content of responses.

For compiler-cli and schematics (which use the TypeScript AST) a major
breaking change was the introduction of the export form:

```typescript
export * as foo from 'bar';
```

This is a `ts.NamespaceExport`, and the `exportClause` of a
`ts.ExportDeclaration` can now take this type as well as `ts.NamedExports`.
This broke a lot of places where `exportClause` was assumed to be
`ts.NamedExports`.

For the most part these breakages were in cases where it is not necessary
to handle the new `ts.NamedExports` anyway. ngtsc's design uses the
`ts.TypeChecker` APIs to understand syntax and so automatically supports the
new form of exports.

The View Engine compiler on the other hand extracts TS structures into
metadata.json files, and that format was not designed for namespaced
exports. As a result it will take a nontrivial amount of work if we want to
support such exports in View Engine. For now, these new exports are not
accounted for in metadata.json, and so using them in "folded" Angular
expressions will result in errors (probably claiming that the referenced
exported namespace doesn't exist).

Care was taken to only use TS APIs which are present in 3.7/3.6, as Angular
needs to remain compatible with these for the time being.

This commit does not update angular.io.

PR Close #35864
2020-03-10 17:51:20 -04:00
Andrew Kushnir
0bf6e58db2 fix(compiler): process imports first and declarations second while calculating scopes (#35850)
Prior to this commit, while calculating the scope for a module, Ivy compiler processed `declarations` field first and `imports` after that. That results in a couple issues:

* for Pipes with the same `name` and present in `declarations` and in an imported module, Pipe from imported module was selected. In View Engine the logic is opposite: Pipes from `declarations` field receive higher priority.
* for Directives with the same selector and present in `declarations` and in an imported module, we first invoked the logic of a Directive from `declarations` field and after that - imported Directive logic. In View Engine, it was the opposite and the logic of a Directive from the `declarations` field was invoked last.

In order to align Ivy and View Engine behavior, this commit updates the logic in which we populate module scope: we first process all imports and after that handle `declarations` field. As a result, in Ivy both use-cases listed above work similar to View Engine.

Resolves #35502.

PR Close #35850
2020-03-10 14:16:59 -04:00
Alex Rickabaugh
983f48136a test(compiler): add a public API guard for the public compiler options (#35885)
This commit adds a public API test which guards against unintentional
changes to the accepted keys in `angularCompilerOptions`.

PR Close #35885
2020-03-10 14:15:28 -04:00
Alex Rickabaugh
edf881dbf1 refactor(compiler): split core/api.ts into multiple files (#35885)
This commit splits the ngtsc `core` package's api entrypoint, which
previously was a single `api.ts` file, into an api/ directory with multiple
files. This is done to isolate the parts of the API definitions pertaining
to the public-facing `angularCompilerOptions` field in tsconfig.json into a
single file, which will enable a public API guard test to be added in a
future commit.

PR Close #35885
2020-03-10 14:15:28 -04:00
Matias Niemelä
15482e7367 Revert "feat(bazel): transform generated shims (in Ivy) with tsickle (#35848)" (#35970)
This reverts commit 9ff9a072e62af9aacc3703936e01c984ad44d6d6.

PR Close #35970
2020-03-09 17:00:14 -04:00
Alex Rickabaugh
9ff9a072e6 feat(bazel): transform generated shims (in Ivy) with tsickle (#35848)
Currently, when Angular code is built with Bazel and with Ivy, generated
factory shims (.ngfactory files) are not processed via the majority of
tsickle's transforms. This is a subtle effect of the build infrastructure,
but it boils down to a TsickleHost method `shouldSkipTsickleProcessing`.

For ngc_wrapped builds (Bazel + Angular), this method is defined in the
`@bazel/typescript` (aka bazel rules_typescript) implementation of
`CompilerHost`. The default behavior is to skip tsickle processing for files
which are not present in the original `srcs[]` of the build rule. In
Angular's case, this includes all generated shim files.

For View Engine factories this is probably desirable as they're quite
complex and they've never been tested with tsickle. Ivy factories however
are smaller and very straightforward, and it makes sense to treat them like
any other output.

This commit adjusts two independent implementations of
`shouldSkipTsickleProcessing` to enable transformation of Ivy shims:

* in `@angular/bazel` aka ngc_wrapped, the upstream `@bazel/typescript`
  `CompilerHost` is patched to treat .ngfactory files the same as their
  original source file, with respect to tsickle processing.

  It is currently not possible to test this change as we don't have any test
  that inspects tsickle output with bazel. It will be extensively tested in
  g3.

* in `ngc`, Angular's own implementation is adjusted to allow for the
  processing of shims when compiling with Ivy. This enables a unit test to
  be written to validate the correct behavior of tsickle when given a host
  that's appropriately configured to process factory shims.

For ngtsc-as-a-plugin, a similar fix will need to be submitted upstream in
tsc_wrapped.

PR Close #35848
2020-03-09 13:06:33 -04:00
Yiting Wang
c296bfcaf9 fix(compiler-cli): suppress extraRequire errors in Closure Compiler (#35737)
This is needed to support https://github.com/angular/tsickle/pull/1133
because it will add an extra require on `tslib`.

PR Close #35737
2020-03-04 08:37:03 -08:00
Alex Rickabaugh
2c41bb8490 fix(compiler): type-checking error for duplicate variables in templates (#35674)
It's an error to declare a variable twice on a specific template:

```html
<div *ngFor="let i of items; let i = index">
</div>
```

This commit introduces a template type-checking error which helps to detect
and diagnose this problem.

Fixes #35186

PR Close #35674
2020-03-03 13:52:50 -08:00
JoostK
3e3a1ef30d fix(ivy): support dynamic query tokens in AOT mode (#35307)
For view and content queries, the Ivy compiler attempts to statically
evaluate the predicate token so that string predicates containing
comma-separated reference names can be split into an array of strings
during compilation. When the predicate is a dynamic value that cannot be
statically interpreted at compile time, the compiler would previously
produce an error. This behavior breaks a use-case where an `InjectionToken`
is being used as query predicate, as the usage of the `new` keyword
prevents such predicates from being statically evaluated.

This commit changes the behavior to no longer produce an error for
dynamic values. Instead, the expression is emitted as is into the
generated code, postponing the evaluation to happen at runtime.

Fixes #34267
Resolves FW-1828

PR Close #35307
2020-02-27 16:05:21 -08:00
Alex Rickabaugh
173a1ac8e4 fix(ivy): better inference for circularly referenced directive types (#35622)
It's possible to pass a directive as an input to itself. Consider:

```html
<some-cmp #ref [value]="ref">
```

Since the template type-checker attempts to infer a type for `<some-cmp>`
using the values of its inputs, this creates a circular reference where the
type of the `value` input is used in its own inference:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: _t0});
```

Obviously, this doesn't work. To resolve this, the template type-checker
used to generate a `null!` expression when a reference would otherwise be
circular:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: null!});
```

This effectively asks TypeScript to infer a value for this context, and
works well to resolve this simple cycle. However, if the template
instead tries to use the circular value in a larger expression:

```html
<some-cmp #ref [value]="ref.prop">
```

The checker would generate:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: (null!).prop});
```

In this case, TypeScript can't figure out any way `null!` could have a
`prop` key, and so it infers `never` as the type. `(never).prop` is thus a
type error.

This commit implements a better fallback pattern for circular references to
directive types like this. Instead of generating a `null!` in place for the
reference, a type is inferred by calling the type constructor again with
`null!` as its input. This infers the widest possible type for the directive
which is then used to break the cycle:

```typescript
var _t0 = SomeCmp.ngTypeCtor(null!);
var _t1 = SomeCmp.ngTypeCtor({value: _t0.prop});
```

This has the desired effect of validating that `.prop` is legal for the
directive type (the type of `#ref`) while also avoiding a cycle.

Fixes #35372
Fixes #35603
Fixes #35522

PR Close #35622
2020-02-26 12:57:08 -08:00
Alex Rickabaugh
2d89b5d13d fix(ivy): provide a more detailed error message for NG6002/NG6003 (#35620)
NG6002/NG6003 are errors produced when an NgModule being compiled has an
imported or exported type which does not have the proper metadata (that is,
it doesn't appear to be an @NgModule, or @Directive, etc. depending on
context).

Previously this error message was a bit sparse. However, Github issues show
that this is the most common error users receive when for whatever reason
ngcc wasn't able to handle one of their libraries, or they just didn't run
it. So this commit changes the error message to offer a bit more useful
context, instructing users differently depending on whether the class in
question is from their own project, from NPM, or from a monorepo-style local
dependency.

PR Close #35620
2020-02-26 12:56:47 -08:00
Alex Eagle
af76651ccc refactor: update tscplugin api to match google3 (#35455)
PR Close #35455
2020-02-24 17:29:33 -08:00
Alex Rickabaugh
4253662231 fix(ivy): add strictLiteralTypes to align Ivy + VE checking of literals (#35462)
Under View Engine's default (non-fullTemplateTypeCheck) checking, object and
array literals which appear in templates are treated as having type `any`.
This allows a number of patterns which would not otherwise compile, such as
indexing an object literal by a string:

```html
{{ {'a': 1, 'b': 2}[value] }}
```

(where `value` is `string`)

Ivy, meanwhile, has always inferred strong types for object literals, even
in its compatibility mode. This commit fixes the bug, and adds the
`strictLiteralTypes` flag to specifically control this inference. When the
flag is `false` (in compatibility mode), object and array literals receive
the `any` type.

PR Close #35462
2020-02-21 12:36:11 -08:00
Alex Rickabaugh
a61fe4177f fix(ivy): emulate a View Engine type-checking bug with safe navigation (#35462)
In its default compatibility mode, the Ivy template type-checker attempts to
emulate the View Engine default mode as accurately as is possible. This
commit addresses a gap in this compatibility that stems from a View Engine
type-checking bug.

Consider two template expressions:

```html
{{ obj?.field }}
{{ fn()?.field }}
```

and suppose that the type of `obj` and `fn()` are the same - both return
either `null` or an object with a `field` property.

Under View Engine, these type-check differently. The `obj` case will catch
if the object type (when not null) does not have a `field` property, while
the `fn()` case will not. This is due to how View Engine represents safe
navigations:

```typescript
// for the 'obj' case
(obj == null ? null as any : obj.field)

// for the 'fn()' case
let tmp: any;
((tmp = fn()) == null ? null as any : tmp.field)
```

Because View Engine uses the same code generation backend as it does to
produce the runtime code for this expression, it uses a ternary for safe
navigation, with a temporary variable to avoid invoking 'fn()' twice. The
type of this temporary variable is 'any', however, which causes the
`tmp.field` check to be meaningless.

Previously, the Ivy template type-checker in compatibility mode assumed that
`fn()?.field` would always check for the presence of 'field' on the non-null
result of `fn()`. This commit emulates the View Engine bug in Ivy's
compatibility mode, so an 'any' type will be inferred under the same
conditions.

As part of this fix, a new format for safe navigation operations in template
type-checking code is introduced. This is based on the realization that
ternary based narrowing is unnecessary.

For the `fn()` case in strict mode, Ivy now generates:

```typescript
(null as any ? fn()!.field : undefined)
```

This effectively uses the ternary operator as a type "or" operation. The
resulting type will be a union of the type of `fn()!.field` with
`undefined`.

For the `fn()` case in compatibility mode, Ivy now emulates the bug with:

```typescript
(fn() as any).field
```

The cast expression includes the call to `fn()` and allows it to be checked
while still returning a type of `any` from the expression.

For the `obj` case in compatibility mode, Ivy now generates:

```typescript
(obj!.field as any)
```

This cast expression still returns `any` for its type, but will check for
the existence of `field` on the type of `obj!`.

PR Close #35462
2020-02-21 12:36:11 -08:00