From 545ec4910a3a92785d95b7f8df7d7ad4a25b7c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20M=C3=BChlena?= Date: Sat, 5 Aug 2023 13:44:30 +0200 Subject: [PATCH 1/4] fix: Ensure tsconfig paths are relative to the workspace root after epxpansion --- gazelle/js/typescript/BUILD.bazel | 2 +- gazelle/js/typescript/tsconfig.go | 49 ++++++++++++++++++------ gazelle/js/typescript/tsconfig_test.go | 52 +++++++++++++++++++++----- 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/gazelle/js/typescript/BUILD.bazel b/gazelle/js/typescript/BUILD.bazel index 9da7edc8c..059224887 100644 --- a/gazelle/js/typescript/BUILD.bazel +++ b/gazelle/js/typescript/BUILD.bazel @@ -9,8 +9,8 @@ go_library( importpath = "aspect.build/cli/gazelle/js/typescript", visibility = ["//visibility:public"], deps = [ + "//gazelle/common/log", "@com_github_msolo_jsonr//:jsonr", - "@com_github_sirupsen_logrus//:logrus", ], ) diff --git a/gazelle/js/typescript/tsconfig.go b/gazelle/js/typescript/tsconfig.go index 81666b434..a0a9f04a1 100644 --- a/gazelle/js/typescript/tsconfig.go +++ b/gazelle/js/typescript/tsconfig.go @@ -1,3 +1,19 @@ +/* + * Copyright 2022 Aspect Build Systems, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package typescript import ( @@ -7,8 +23,8 @@ import ( "sort" "strings" + . "aspect.build/cli/gazelle/common/log" "github.com/msolo/jsonr" - "github.com/sirupsen/logrus" ) type tsCompilerOptionsJSON struct { @@ -44,8 +60,6 @@ var DefaultConfigPaths = TsConfigPaths{ Map: &map[string][]string{}, } -var Log = logrus.New() - // parseTsConfigJSONFile loads a tsconfig.json file and return the compilerOptions config func parseTsConfigJSONFile(cm *TsConfigMap, root, dir, tsconfig string) (*TsConfig, error) { existing := cm.configs[dir] @@ -73,7 +87,7 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent if c.Extends != "" { base, err := parseTsConfigJSONFile(cm, root, path.Join(configDir, path.Dir(c.Extends)), path.Base(c.Extends)) if err != nil { - Log.Warnf("Failed to load base tsconfig file %s: %v", path.Join(configDir, c.Extends), err) + BazelLog.Warnf("Failed to load base tsconfig file %s: %v", path.Join(configDir, c.Extends), err) } baseConfig = base @@ -83,7 +97,7 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent if baseConfig != nil { rel, relErr := filepath.Rel(configDir, baseConfig.ConfigDir) if relErr != nil { - Log.Warnf("Failed to resolve relative path from %s to %s: %v", configDir, baseConfig.ConfigDir, relErr) + BazelLog.Warnf("Failed to resolve relative path from %s to %s: %v", configDir, baseConfig.ConfigDir, relErr) } else { baseConfigRel = rel } @@ -140,6 +154,19 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent return &config, nil } +// Returns the path from the bazel-root to the active tsconfig.json file +// if the passed path is not absolute. +// Or an empty string if the path is absolute +func (c TsConfig) getRelativeExpansionIfLocal(importPath string) string { + // Absolute paths must never be expanded but everything else must be relative to the bazel-root + // and therefore expanded with the path to the current active tsconfig.json + if !path.IsAbs(importPath) { + BazelLog.Debugf("Found local path %s in tsconfig.json. Should be expanded with tsconfig dir: %s", importPath, c.ConfigDir) + return c.ConfigDir + } + return "" +} + // Expand the given path to all possible mapped paths for this config, in priority order. // // Path matching algorithm based on ESBuild implementation @@ -151,10 +178,8 @@ func (c TsConfig) ExpandPaths(from, p string) []string { // Check for exact 'paths' matches first exact := (*pathMap)[p] - if exact != nil { - for _, m := range exact { - possible = append(possible, path.Clean(path.Join(c.Paths.Rel, m))) - } + for _, m := range exact { + possible = append(possible, path.Clean(path.Join(c.getRelativeExpansionIfLocal(m), c.Paths.Rel, m))) } // Check for pattern matches next @@ -183,12 +208,12 @@ func (c TsConfig) ExpandPaths(from, p string) []string { matchedText := p[len(m.prefix) : len(p)-len(m.suffix)] mappedPath := strings.Replace(originalPath, "*", matchedText, 1) - mappedPath = path.Clean(mappedPath) - - possible = append(possible, path.Join(c.Paths.Rel, mappedPath)) + possible = append(possible, path.Join(c.getRelativeExpansionIfLocal(mappedPath), c.Paths.Rel, mappedPath)) } } + BazelLog.Tracef("Found %d possible paths for %s: %v", len(possible), p, possible) + // Add 'rootDirs' as alternate directories for relative imports // https://www.typescriptlang.org/tsconfig#rootDirs for _, v := range c.VirtualRootDirs { diff --git a/gazelle/js/typescript/tsconfig_test.go b/gazelle/js/typescript/tsconfig_test.go index b313c8a0c..f7327bcef 100644 --- a/gazelle/js/typescript/tsconfig_test.go +++ b/gazelle/js/typescript/tsconfig_test.go @@ -1,3 +1,19 @@ +/* + * Copyright 2022 Aspect Build Systems, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package typescript import ( @@ -96,6 +112,22 @@ func TestTypescriptApi(t *testing.T) { `) }) + t.Run("tsconfig paths inheritance", func(t *testing.T) { + + // Mock a config manually to set a custom Rel path (like an external tsconfig was loaded) + config := &TsConfig{ + ConfigDir: "tsconfig_test", + Paths: &TsConfigPaths{ + Rel: "../libs/ts/liba", + Map: &map[string][]string{ + "@org/liba/*": {"src/*"}, + }, + }, + } + + assertExpand(t, config, "@org/liba/test", "libs/ts/liba/src/test") + }) + t.Run("tsconfig paths expansion basic", func(t *testing.T) { // Initial request: https://github.com/aspect-build/aspect-cli/issues/396 config := parseTest(t, `{ @@ -110,7 +142,7 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "@org/lib", "b/src/lib") + assertExpand(t, config, "@org/lib", "tsconfig_test/b/src/lib") }) t.Run("tsconfig paths expansion", func(t *testing.T) { @@ -128,17 +160,17 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "test0", "test0-success.ts") - assertExpand(t, config, "test1/bar", "test1-success.ts") - assertExpand(t, config, "test1/foo", "test1-success.ts") - assertExpand(t, config, "test2/foo", "test2-success/foo") + assertExpand(t, config, "test0", "tsconfig_test/test0-success.ts") + assertExpand(t, config, "test1/bar", "tsconfig_test/test1-success.ts") + assertExpand(t, config, "test1/foo", "tsconfig_test/test1-success.ts") + assertExpand(t, config, "test2/foo", "tsconfig_test/test2-success/foo") assertExpand(t, config, "test3/x") - assertExpand(t, config, "tXt3/foo", "test3-succXs.ts") - assertExpand(t, config, "t123t3/foo", "test3-succ123s.ts") - assertExpand(t, config, "t-t3/foo", "test3-succ-s.ts") + assertExpand(t, config, "tXt3/foo", "tsconfig_test/test3-succXs.ts") + assertExpand(t, config, "t123t3/foo", "tsconfig_test/test3-succ123s.ts") + assertExpand(t, config, "t-t3/foo", "tsconfig_test/test3-succ-s.ts") - assertExpand(t, config, "test4/x", "test4-first/x", "test4-second/x") + assertExpand(t, config, "test4/x", "tsconfig_test/test4-first/x", "tsconfig_test/test4-second/x") }) t.Run("tsconfig paths expansion star-length tie-breaker", func(t *testing.T) { @@ -156,6 +188,6 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "lib/a", "a-direct", "fallback/a", "lib-star/a", "li-star/b/a", "l-star/ib/a") + assertExpand(t, config, "lib/a", "tsconfig_test/a-direct", "tsconfig_test/fallback/a", "tsconfig_test/lib-star/a", "tsconfig_test/li-star/b/a", "tsconfig_test/l-star/ib/a") }) } From 5b27f83d51274c10378ef4b519b4f59856b93197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20M=C3=BChlena?= Date: Sat, 5 Aug 2023 13:47:48 +0200 Subject: [PATCH 2/4] test: gazelle generations tests for expansion --- gazelle/js/tests/tsconfig_paths/custom/BUILD.in | 0 gazelle/js/tests/tsconfig_paths/custom/BUILD.out | 13 +++++++++++++ gazelle/js/tests/tsconfig_paths/custom/main.ts | 10 ++++++++++ .../js/tests/tsconfig_paths/custom/nested/BUILD.in | 0 .../js/tests/tsconfig_paths/custom/nested/BUILD.out | 7 +++++++ .../js/tests/tsconfig_paths/custom/nested/test.ts | 5 +++++ .../js/tests/tsconfig_paths/custom/tsconfig.json | 13 +++++++++++++ gazelle/js/tests/tsconfig_paths/tsconfig.json | 3 ++- 8 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 gazelle/js/tests/tsconfig_paths/custom/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_paths/custom/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_paths/custom/main.ts create mode 100644 gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_paths/custom/nested/test.ts create mode 100644 gazelle/js/tests/tsconfig_paths/custom/tsconfig.json diff --git a/gazelle/js/tests/tsconfig_paths/custom/BUILD.in b/gazelle/js/tests/tsconfig_paths/custom/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_paths/custom/BUILD.out b/gazelle/js/tests/tsconfig_paths/custom/BUILD.out new file mode 100644 index 000000000..a548d89e7 --- /dev/null +++ b/gazelle/js/tests/tsconfig_paths/custom/BUILD.out @@ -0,0 +1,13 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "custom", + srcs = ["main.ts"], + deps = [ + "//custom/nested", + "//fallback", + "//lib/a", + "//lib/b", + "//lib/c", + ], +) diff --git a/gazelle/js/tests/tsconfig_paths/custom/main.ts b/gazelle/js/tests/tsconfig_paths/custom/main.ts new file mode 100644 index 000000000..00cff81e8 --- /dev/null +++ b/gazelle/js/tests/tsconfig_paths/custom/main.ts @@ -0,0 +1,10 @@ +import { A } from 'star/a'; +import { B } from 'star/b'; +import { A as AliasA } from 'alias-a'; +import { A as RootDotA } from 'lib/a'; +import { C1 } from 'multi-c/c1'; +import { C2 } from 'multi-c/c2'; +import { F } from 'f1'; +import { Test } from '@/nested/test'; + +console.log(A, B, AliasA, RootDotA, C1, C2, F, Test); diff --git a/gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.in b/gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.out b/gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.out new file mode 100644 index 000000000..561c7ddf2 --- /dev/null +++ b/gazelle/js/tests/tsconfig_paths/custom/nested/BUILD.out @@ -0,0 +1,7 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "nested", + srcs = ["test.ts"], + deps = ["//lib/a"], +) diff --git a/gazelle/js/tests/tsconfig_paths/custom/nested/test.ts b/gazelle/js/tests/tsconfig_paths/custom/nested/test.ts new file mode 100644 index 000000000..40e699032 --- /dev/null +++ b/gazelle/js/tests/tsconfig_paths/custom/nested/test.ts @@ -0,0 +1,5 @@ +import { A as AliasA } from 'alias-a'; + +console.log(AliasA); + +export const Test = 'Test'; diff --git a/gazelle/js/tests/tsconfig_paths/custom/tsconfig.json b/gazelle/js/tests/tsconfig_paths/custom/tsconfig.json new file mode 100644 index 000000000..7f8fb589f --- /dev/null +++ b/gazelle/js/tests/tsconfig_paths/custom/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "baseUrl": ".", + "paths": { + "alias-a": ["../lib/a"], + "star/*": ["../lib/*"], + "multi-c/*": ["../lib/c/first/*", "../lib/c/second/*"], + "@/*": ["*"], + "*": ["../fallback/*", "../*"] + } + }, + "include": ["**/*.ts"] +} diff --git a/gazelle/js/tests/tsconfig_paths/tsconfig.json b/gazelle/js/tests/tsconfig_paths/tsconfig.json index bbb24b9c9..e72826e1a 100644 --- a/gazelle/js/tests/tsconfig_paths/tsconfig.json +++ b/gazelle/js/tests/tsconfig_paths/tsconfig.json @@ -7,5 +7,6 @@ "multi-c/*": ["./lib/c/first/*", "./lib/c/second/*"], "*": ["./fallback/*", "*"] } - } + }, + "include": ["./src/**/*.ts"] } From d773a2e249f5f2fbf6e5c04cf7a40dbc4d20914b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20M=C3=BChlena?= Date: Mon, 7 Aug 2023 20:15:13 +0200 Subject: [PATCH 3/4] feat: add baseUrl based possibilities to import path expansion --- gazelle/js/tests/tsconfig_baseurl/BUILD.in | 1 + gazelle/js/tests/tsconfig_baseurl/BUILD.out | 13 ++++ gazelle/js/tests/tsconfig_baseurl/WORKSPACE | 2 + .../tests/tsconfig_baseurl/inheriter/BUILD.in | 0 .../tsconfig_baseurl/inheriter/BUILD.out | 10 ++++ .../tests/tsconfig_baseurl/inheriter/main.ts | 4 ++ .../tsconfig_baseurl/inheriter/tsconfig.json | 4 ++ .../js/tests/tsconfig_baseurl/lib/a/BUILD.in | 0 .../js/tests/tsconfig_baseurl/lib/a/BUILD.out | 6 ++ .../js/tests/tsconfig_baseurl/lib/a/index.ts | 1 + .../js/tests/tsconfig_baseurl/lib/b/BUILD.in | 0 .../js/tests/tsconfig_baseurl/lib/b/BUILD.out | 6 ++ .../js/tests/tsconfig_baseurl/lib/b/index.ts | 1 + .../js/tests/tsconfig_baseurl/lib/c/BUILD.in | 0 .../js/tests/tsconfig_baseurl/lib/c/BUILD.out | 9 +++ .../tests/tsconfig_baseurl/lib/c/first/c1.ts | 1 + .../tests/tsconfig_baseurl/lib/c/second/c2.ts | 1 + .../tsconfig_baseurl/projects/p1/BUILD.in | 1 + .../tsconfig_baseurl/projects/p1/BUILD.out | 9 +++ .../tsconfig_baseurl/projects/p1/index.ts | 3 + .../projects/p1/nested_lib/a/BUILD.in | 1 + .../projects/p1/nested_lib/a/BUILD.out | 8 +++ .../projects/p1/nested_lib/a/a.ts | 1 + .../projects/p1/tsconfig.json | 6 ++ .../js/tests/tsconfig_baseurl/src/index.ts | 6 ++ .../js/tests/tsconfig_baseurl/tsconfig.json | 6 ++ gazelle/js/typescript/tsconfig.go | 15 ++++- gazelle/js/typescript/tsconfig_test.go | 59 +++++++++++++++---- 28 files changed, 163 insertions(+), 11 deletions(-) create mode 100644 gazelle/js/tests/tsconfig_baseurl/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/WORKSPACE create mode 100644 gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/inheriter/main.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/inheriter/tsconfig.json create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/a/index.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/b/index.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/c/first/c1.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/lib/c/second/c2.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/index.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.in create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.out create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/a.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/projects/p1/tsconfig.json create mode 100644 gazelle/js/tests/tsconfig_baseurl/src/index.ts create mode 100644 gazelle/js/tests/tsconfig_baseurl/tsconfig.json diff --git a/gazelle/js/tests/tsconfig_baseurl/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/BUILD.in new file mode 100644 index 000000000..2b0ce41a9 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/BUILD.in @@ -0,0 +1 @@ +# gazelle:js_generation_mode none \ No newline at end of file diff --git a/gazelle/js/tests/tsconfig_baseurl/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/BUILD.out new file mode 100644 index 000000000..2cc332df6 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/BUILD.out @@ -0,0 +1,13 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +# gazelle:js_generation_mode none + +ts_project( + name = "tsconfig_rootdir", + srcs = ["src/index.ts"], + deps = [ + "//lib/a", + "//lib/b", + "//lib/c", + ], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/WORKSPACE b/gazelle/js/tests/tsconfig_baseurl/WORKSPACE new file mode 100644 index 000000000..f42135673 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/WORKSPACE @@ -0,0 +1,2 @@ +# This is a Bazel workspace for the Gazelle test data. +workspace(name = "tsconfig_rootdir") diff --git a/gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.out new file mode 100644 index 000000000..ef6ac8a5f --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/inheriter/BUILD.out @@ -0,0 +1,10 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "inheriter", + srcs = ["main.ts"], + deps = [ + "//lib/a", + "//lib/c", + ], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/inheriter/main.ts b/gazelle/js/tests/tsconfig_baseurl/inheriter/main.ts new file mode 100644 index 000000000..be68af7bb --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/inheriter/main.ts @@ -0,0 +1,4 @@ +import { A } from 'lib/a'; +import { C1 } from 'lib/c/first/c1'; + +console.log(A, C1); diff --git a/gazelle/js/tests/tsconfig_baseurl/inheriter/tsconfig.json b/gazelle/js/tests/tsconfig_baseurl/inheriter/tsconfig.json new file mode 100644 index 000000000..5197ce276 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/inheriter/tsconfig.json @@ -0,0 +1,4 @@ +{ + "extends": "../tsconfig.json", + "include": ["./**/*.ts"] +} diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.out new file mode 100644 index 000000000..ff508be2e --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/a/BUILD.out @@ -0,0 +1,6 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "a", + srcs = ["index.ts"], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/a/index.ts b/gazelle/js/tests/tsconfig_baseurl/lib/a/index.ts new file mode 100644 index 000000000..80e9f2278 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/a/index.ts @@ -0,0 +1 @@ +export const A = 'a'; diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.out new file mode 100644 index 000000000..cc12aaf0a --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/b/BUILD.out @@ -0,0 +1,6 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "b", + srcs = ["index.ts"], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/b/index.ts b/gazelle/js/tests/tsconfig_baseurl/lib/b/index.ts new file mode 100644 index 000000000..28c4501eb --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/b/index.ts @@ -0,0 +1 @@ +export const B = 'b'; diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.in new file mode 100644 index 000000000..e69de29bb diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.out new file mode 100644 index 000000000..32da5c05b --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/c/BUILD.out @@ -0,0 +1,9 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +ts_project( + name = "c", + srcs = [ + "first/c1.ts", + "second/c2.ts", + ], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/c/first/c1.ts b/gazelle/js/tests/tsconfig_baseurl/lib/c/first/c1.ts new file mode 100644 index 000000000..f9a54e95f --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/c/first/c1.ts @@ -0,0 +1 @@ +export const C1 = 'c1'; diff --git a/gazelle/js/tests/tsconfig_baseurl/lib/c/second/c2.ts b/gazelle/js/tests/tsconfig_baseurl/lib/c/second/c2.ts new file mode 100644 index 000000000..96b8d7a36 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/lib/c/second/c2.ts @@ -0,0 +1 @@ +export const C2 = 'c2'; diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.in new file mode 100644 index 000000000..2b0ce41a9 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.in @@ -0,0 +1 @@ +# gazelle:js_generation_mode none \ No newline at end of file diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.out new file mode 100644 index 000000000..4f95c39a7 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/BUILD.out @@ -0,0 +1,9 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +# gazelle:js_generation_mode none + +ts_project( + name = "p1", + srcs = ["index.ts"], + deps = ["//projects/p1/nested_lib/a"], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/index.ts b/gazelle/js/tests/tsconfig_baseurl/projects/p1/index.ts new file mode 100644 index 000000000..612ca3941 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/index.ts @@ -0,0 +1,3 @@ +import { NestedA } from 'nested_lib/a/a'; + +console.log(NestedA); diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.in b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.in new file mode 100644 index 000000000..2b0ce41a9 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.in @@ -0,0 +1 @@ +# gazelle:js_generation_mode none \ No newline at end of file diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.out b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.out new file mode 100644 index 000000000..44feece34 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/BUILD.out @@ -0,0 +1,8 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") + +# gazelle:js_generation_mode none + +ts_project( + name = "a", + srcs = ["a.ts"], +) diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/a.ts b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/a.ts new file mode 100644 index 000000000..dfca88662 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/nested_lib/a/a.ts @@ -0,0 +1 @@ +export const NestedA = 'internal lib'; diff --git a/gazelle/js/tests/tsconfig_baseurl/projects/p1/tsconfig.json b/gazelle/js/tests/tsconfig_baseurl/projects/p1/tsconfig.json new file mode 100644 index 000000000..a7a1b6e73 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/projects/p1/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "baseUrl": "." + }, + "include": ["./**/*.ts"] +} diff --git a/gazelle/js/tests/tsconfig_baseurl/src/index.ts b/gazelle/js/tests/tsconfig_baseurl/src/index.ts new file mode 100644 index 000000000..dac5471a7 --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/src/index.ts @@ -0,0 +1,6 @@ +import { A } from '../lib/a'; +import { B } from '../lib/b'; +import { C1 } from '../lib/c/first/c1'; +import { C2 } from '../lib/c/second/c2'; + +console.log(A, B, C1, C2); diff --git a/gazelle/js/tests/tsconfig_baseurl/tsconfig.json b/gazelle/js/tests/tsconfig_baseurl/tsconfig.json new file mode 100644 index 000000000..d6a20461a --- /dev/null +++ b/gazelle/js/tests/tsconfig_baseurl/tsconfig.json @@ -0,0 +1,6 @@ +{ + "compilerOptions": { + "baseUrl": "." + }, + "include": ["./src/**/*.ts"] +} diff --git a/gazelle/js/typescript/tsconfig.go b/gazelle/js/typescript/tsconfig.go index a0a9f04a1..19e29eb9f 100644 --- a/gazelle/js/typescript/tsconfig.go +++ b/gazelle/js/typescript/tsconfig.go @@ -20,6 +20,7 @@ import ( "os" "path" "path/filepath" + "regexp" "sort" "strings" @@ -60,6 +61,10 @@ var DefaultConfigPaths = TsConfigPaths{ Map: &map[string][]string{}, } +// Matches strings starting with ../, ./, @ or . +// Used to detect if a path could possibly expanded using baseUrl +var baseurl_validation_regex = regexp.MustCompile(`^(\.\.?\/|@|\.)`) + // parseTsConfigJSONFile loads a tsconfig.json file and return the compilerOptions config func parseTsConfigJSONFile(cm *TsConfigMap, root, dir, tsconfig string) (*TsConfig, error) { existing := cm.configs[dir] @@ -113,6 +118,8 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent var BaseUrl string if c.CompilerOptions.BaseUrl != nil { BaseUrl = path.Clean(*c.CompilerOptions.BaseUrl) + } else if baseConfig != nil && baseConfig.BaseUrl != "" { + BaseUrl = path.Join(baseConfigRel, baseConfig.BaseUrl) } else { BaseUrl = "." } @@ -173,7 +180,6 @@ func (c TsConfig) getRelativeExpansionIfLocal(importPath string) string { // Inspired by: https://github.com/evanw/esbuild/blob/deb93e92267a96575a6e434ff18421f4ef0605e4/internal/resolver/resolver.go#L1831-L1945 func (c TsConfig) ExpandPaths(from, p string) []string { pathMap := c.Paths.Map - possible := make([]string, 0) // Check for exact 'paths' matches first @@ -212,6 +218,13 @@ func (c TsConfig) ExpandPaths(from, p string) []string { } } + // Expand paths from baseUrl + // Must not to be absolute or relative to be expanded + // https://www.typescriptlang.org/tsconfig#baseUrl + if !baseurl_validation_regex.MatchString(p) && !path.IsAbs(p) { + possible = append(possible, path.Join(c.getRelativeExpansionIfLocal(p), c.BaseUrl, p)) + } + BazelLog.Tracef("Found %d possible paths for %s: %v", len(possible), p, possible) // Add 'rootDirs' as alternate directories for relative imports diff --git a/gazelle/js/typescript/tsconfig_test.go b/gazelle/js/typescript/tsconfig_test.go index f7327bcef..754dd565a 100644 --- a/gazelle/js/typescript/tsconfig_test.go +++ b/gazelle/js/typescript/tsconfig_test.go @@ -47,6 +47,45 @@ func assertExpand(t *testing.T, options *TsConfig, p string, expected ...string) } } +func TestBaseUrlRegex(t *testing.T) { + t.Parallel() + + t.Run("valid strings", func(t *testing.T) { + t.Parallel() + + shouldNotMatch := []string{ + "example", + "valid", + "another", + } + + for _, s := range shouldNotMatch { + if baseurl_validation_regex.MatchString(s) { + t.Errorf("Expected no match for '%s', but it matched", s) + } + } + + }) + + t.Run("invalid strings", func(t *testing.T) { + t.Parallel() + + shouldMatch := []string{ + "./path", + "../parent", + "@username", + ".invalid", + } + + for _, s := range shouldMatch { + if !baseurl_validation_regex.MatchString(s) { + t.Errorf("Expected a match for '%s', but it didn't match", s) + } + } + }) + +} + func TestTypescriptApi(t *testing.T) { t.Run("parse a tsconfig with empty config", func(t *testing.T) { options := parseTest(t, "{}") @@ -160,17 +199,17 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "test0", "tsconfig_test/test0-success.ts") - assertExpand(t, config, "test1/bar", "tsconfig_test/test1-success.ts") - assertExpand(t, config, "test1/foo", "tsconfig_test/test1-success.ts") - assertExpand(t, config, "test2/foo", "tsconfig_test/test2-success/foo") - assertExpand(t, config, "test3/x") + assertExpand(t, config, "test0", "tsconfig_test/test0-success.ts", "tsconfig_test/test0") + assertExpand(t, config, "test1/bar", "tsconfig_test/test1-success.ts", "tsconfig_test/test1/bar") + assertExpand(t, config, "test1/foo", "tsconfig_test/test1-success.ts", "tsconfig_test/test1/foo") + assertExpand(t, config, "test2/foo", "tsconfig_test/test2-success/foo", "tsconfig_test/test2/foo") + assertExpand(t, config, "test3/x", "tsconfig_test/test3/x") - assertExpand(t, config, "tXt3/foo", "tsconfig_test/test3-succXs.ts") - assertExpand(t, config, "t123t3/foo", "tsconfig_test/test3-succ123s.ts") - assertExpand(t, config, "t-t3/foo", "tsconfig_test/test3-succ-s.ts") + assertExpand(t, config, "tXt3/foo", "tsconfig_test/test3-succXs.ts", "tsconfig_test/tXt3/foo") + assertExpand(t, config, "t123t3/foo", "tsconfig_test/test3-succ123s.ts", "tsconfig_test/t123t3/foo") + assertExpand(t, config, "t-t3/foo", "tsconfig_test/test3-succ-s.ts", "tsconfig_test/t-t3/foo") - assertExpand(t, config, "test4/x", "tsconfig_test/test4-first/x", "tsconfig_test/test4-second/x") + assertExpand(t, config, "test4/x", "tsconfig_test/test4-first/x", "tsconfig_test/test4-second/x", "tsconfig_test/test4/x") }) t.Run("tsconfig paths expansion star-length tie-breaker", func(t *testing.T) { @@ -188,6 +227,6 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "lib/a", "tsconfig_test/a-direct", "tsconfig_test/fallback/a", "tsconfig_test/lib-star/a", "tsconfig_test/li-star/b/a", "tsconfig_test/l-star/ib/a") + assertExpand(t, config, "lib/a", "tsconfig_test/a-direct", "tsconfig_test/fallback/a", "tsconfig_test/lib-star/a", "tsconfig_test/li-star/b/a", "tsconfig_test/l-star/ib/a", "tsconfig_test/lib/a") }) } From 19558e37236c09e1156060f237071275613f312c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joel=20M=C3=BChlena?= Date: Thu, 10 Aug 2023 11:10:00 +0200 Subject: [PATCH 4/4] fix: review changes --- gazelle/js/typescript/tsconfig.go | 33 ++++++++++++++------------ gazelle/js/typescript/tsconfig_test.go | 32 ++++++++++--------------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/gazelle/js/typescript/tsconfig.go b/gazelle/js/typescript/tsconfig.go index 19e29eb9f..827f8c899 100644 --- a/gazelle/js/typescript/tsconfig.go +++ b/gazelle/js/typescript/tsconfig.go @@ -20,7 +20,6 @@ import ( "os" "path" "path/filepath" - "regexp" "sort" "strings" @@ -61,9 +60,13 @@ var DefaultConfigPaths = TsConfigPaths{ Map: &map[string][]string{}, } -// Matches strings starting with ../, ./, @ or . -// Used to detect if a path could possibly expanded using baseUrl -var baseurl_validation_regex = regexp.MustCompile(`^(\.\.?\/|@|\.)`) +func isRelativePath(p string) bool { + if path.IsAbs(p) { + return false + } + + return strings.HasPrefix(p, "./") || strings.HasPrefix(p, "../") +} // parseTsConfigJSONFile loads a tsconfig.json file and return the compilerOptions config func parseTsConfigJSONFile(cm *TsConfigMap, root, dir, tsconfig string) (*TsConfig, error) { @@ -118,8 +121,6 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent var BaseUrl string if c.CompilerOptions.BaseUrl != nil { BaseUrl = path.Clean(*c.CompilerOptions.BaseUrl) - } else if baseConfig != nil && baseConfig.BaseUrl != "" { - BaseUrl = path.Join(baseConfigRel, baseConfig.BaseUrl) } else { BaseUrl = "." } @@ -161,14 +162,16 @@ func parseTsConfigJSON(cm *TsConfigMap, root, configDir string, tsconfigContent return &config, nil } -// Returns the path from the bazel-root to the active tsconfig.json file +// Returns the path from the project base to the active tsconfig.json file +// This is used to build the path from the project base to the file being imported +// because gazelle seems to resolve files relative to the project base // if the passed path is not absolute. // Or an empty string if the path is absolute -func (c TsConfig) getRelativeExpansionIfLocal(importPath string) string { +func (c TsConfig) expandRelativePath(importPath string) string { // Absolute paths must never be expanded but everything else must be relative to the bazel-root // and therefore expanded with the path to the current active tsconfig.json if !path.IsAbs(importPath) { - BazelLog.Debugf("Found local path %s in tsconfig.json. Should be expanded with tsconfig dir: %s", importPath, c.ConfigDir) + BazelLog.Tracef("Found local path %s in tsconfig.json. Should be expanded with tsconfig dir: %s", importPath, c.ConfigDir) return c.ConfigDir } return "" @@ -185,7 +188,7 @@ func (c TsConfig) ExpandPaths(from, p string) []string { // Check for exact 'paths' matches first exact := (*pathMap)[p] for _, m := range exact { - possible = append(possible, path.Clean(path.Join(c.getRelativeExpansionIfLocal(m), c.Paths.Rel, m))) + possible = append(possible, path.Join(c.expandRelativePath(m), c.Paths.Rel, m)) } // Check for pattern matches next @@ -214,25 +217,25 @@ func (c TsConfig) ExpandPaths(from, p string) []string { matchedText := p[len(m.prefix) : len(p)-len(m.suffix)] mappedPath := strings.Replace(originalPath, "*", matchedText, 1) - possible = append(possible, path.Join(c.getRelativeExpansionIfLocal(mappedPath), c.Paths.Rel, mappedPath)) + possible = append(possible, path.Join(c.expandRelativePath(mappedPath), c.Paths.Rel, mappedPath)) } } // Expand paths from baseUrl // Must not to be absolute or relative to be expanded // https://www.typescriptlang.org/tsconfig#baseUrl - if !baseurl_validation_regex.MatchString(p) && !path.IsAbs(p) { - possible = append(possible, path.Join(c.getRelativeExpansionIfLocal(p), c.BaseUrl, p)) + if !isRelativePath(p) { + possible = append(possible, path.Join(c.expandRelativePath(p), c.BaseUrl, p)) } - BazelLog.Tracef("Found %d possible paths for %s: %v", len(possible), p, possible) - // Add 'rootDirs' as alternate directories for relative imports // https://www.typescriptlang.org/tsconfig#rootDirs for _, v := range c.VirtualRootDirs { possible = append(possible, path.Join(v, p)) } + BazelLog.Tracef("Found %d possible paths for %s: %v", len(possible), p, possible) + return possible } diff --git a/gazelle/js/typescript/tsconfig_test.go b/gazelle/js/typescript/tsconfig_test.go index 754dd565a..c3db67bde 100644 --- a/gazelle/js/typescript/tsconfig_test.go +++ b/gazelle/js/typescript/tsconfig_test.go @@ -47,39 +47,33 @@ func assertExpand(t *testing.T, options *TsConfig, p string, expected ...string) } } -func TestBaseUrlRegex(t *testing.T) { - t.Parallel() - - t.Run("valid strings", func(t *testing.T) { - t.Parallel() +func TestIsRelativePath(t *testing.T) { + t.Run("relative path strings", func(t *testing.T) { shouldNotMatch := []string{ - "example", - "valid", - "another", + "example/test", + "/absolute/path", + "another/not/relative/path", + ".dotfile", } for _, s := range shouldNotMatch { - if baseurl_validation_regex.MatchString(s) { - t.Errorf("Expected no match for '%s', but it matched", s) + if isRelativePath(s) { + t.Errorf("isRelativePath(%s) should not be relative but was matched as it would", s) } } }) - t.Run("invalid strings", func(t *testing.T) { - t.Parallel() - + t.Run("not relative path strings", func(t *testing.T) { shouldMatch := []string{ "./path", "../parent", - "@username", - ".invalid", } for _, s := range shouldMatch { - if !baseurl_validation_regex.MatchString(s) { - t.Errorf("Expected a match for '%s', but it didn't match", s) + if !isRelativePath(s) { + t.Errorf("isRelativePath(%s) should be relative but was NOT matched as it would", s) } } }) @@ -164,7 +158,7 @@ func TestTypescriptApi(t *testing.T) { }, } - assertExpand(t, config, "@org/liba/test", "libs/ts/liba/src/test") + assertExpand(t, config, "@org/liba/test", "libs/ts/liba/src/test", "tsconfig_test/@org/liba/test") }) t.Run("tsconfig paths expansion basic", func(t *testing.T) { @@ -181,7 +175,7 @@ func TestTypescriptApi(t *testing.T) { } }`) - assertExpand(t, config, "@org/lib", "tsconfig_test/b/src/lib") + assertExpand(t, config, "@org/lib", "tsconfig_test/b/src/lib", "tsconfig_test/@org/lib") }) t.Run("tsconfig paths expansion", func(t *testing.T) {