Skip to content

Commit cad7ae0

Browse files
kuhekuhe
authored
fix: check dependencies when adding imports (#1239)
* fix: check dependencies when adding imports * avoid method overload with super type * handle node: prefix package imports * exempt node: prefix packages from enforced registration * remove addImportUnchecked * remove stray deprecation tag * convert import logic to class * comment grammar * add internalapi annotation to ImportFrom
1 parent ca9cb13 commit cad7ae0

File tree

4 files changed

+226
-5
lines changed

4 files changed

+226
-5
lines changed

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptWriter.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.util.function.UnaryOperator;
2121
import software.amazon.smithy.codegen.core.CodegenException;
2222
import software.amazon.smithy.codegen.core.Symbol;
23+
import software.amazon.smithy.codegen.core.SymbolDependency;
2324
import software.amazon.smithy.codegen.core.SymbolReference;
2425
import software.amazon.smithy.codegen.core.SymbolWriter;
2526
import software.amazon.smithy.model.Model;
@@ -29,6 +30,7 @@
2930
import software.amazon.smithy.model.traits.DeprecatedTrait;
3031
import software.amazon.smithy.model.traits.DocumentationTrait;
3132
import software.amazon.smithy.model.traits.InternalTrait;
33+
import software.amazon.smithy.typescript.codegen.validation.ImportFrom;
3234
import software.amazon.smithy.utils.SmithyUnstableApi;
3335
import software.amazon.smithy.utils.StringUtils;
3436

@@ -114,19 +116,40 @@ public TypeScriptWriter addIgnoredDefaultImport(String name, String from, String
114116
*/
115117
@Deprecated
116118
public TypeScriptWriter addImport(String name, String as, String from) {
119+
ImportFrom importFrom = new ImportFrom(from);
120+
121+
if (importFrom.isDeclarablePackageImport()) {
122+
String packageName = importFrom.getPackageName();
123+
if (getDependencies()
124+
.stream()
125+
.map(SymbolDependency::getPackageName)
126+
.noneMatch(packageName::equals)) {
127+
throw new CodegenException(
128+
"""
129+
The import %s does not correspond to a registered dependency.
130+
TypeScriptWriter::addDependency() is required before ::addImport().
131+
""".formatted(from)
132+
);
133+
}
134+
}
135+
117136
getImportContainer().addImport(name, as, from);
118137
return this;
119138
}
120139

121140
/**
122141
* Imports a type using an alias from a module only if necessary.
142+
* Adds the dependency.
123143
*
124144
* @param name Type to import.
125145
* @param as Alias to refer to the type as.
126146
* @param from PackageContainer to import the type from.
127147
* @return Returns the writer.
128148
*/
129149
public TypeScriptWriter addImport(String name, String as, PackageContainer from) {
150+
if (from instanceof Dependency) {
151+
addDependency((Dependency) from);
152+
}
130153
return this.addImport(name, as, from.getPackageName());
131154
}
132155

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/validation/ImportFrom.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package software.amazon.smithy.typescript.codegen.validation;
7+
8+
import java.util.Set;
9+
import software.amazon.smithy.utils.SetUtils;
10+
import software.amazon.smithy.utils.SmithyInternalApi;
11+
12+
/**
13+
* Interprets the string portion of an import statement.
14+
*/
15+
@SmithyInternalApi
16+
public class ImportFrom {
17+
public static final Set<String> NODE_NATIVE_DEPENDENCIES = SetUtils.of(
18+
"buffer",
19+
"child_process",
20+
"crypto",
21+
"dns",
22+
"events",
23+
"fs",
24+
"http",
25+
"http2",
26+
"https",
27+
"os",
28+
"path",
29+
"process",
30+
"stream",
31+
"tls",
32+
"url",
33+
"util",
34+
"zlib"
35+
);
36+
37+
private final String from;
38+
39+
public ImportFrom(String importTargetExpression) {
40+
this.from = importTargetExpression;
41+
}
42+
43+
/**
44+
* @return whether we recognize it as a Node.js native module. These
45+
* do not need to be declared in package.json. This check
46+
* is not exhaustive since the list of native modules varies
47+
* by version.
48+
*/
49+
public boolean isNodejsNative() {
50+
String[] packageNameSegments = from.split("/");
51+
return from.startsWith("node:")
52+
|| NODE_NATIVE_DEPENDENCIES.contains(packageNameSegments[0]);
53+
}
54+
55+
/**
56+
* @return whether the import has an org or namespace prefix like \@smithy/pkg.
57+
*/
58+
public boolean isNamespaced() {
59+
return from.startsWith("@") && from.contains("/");
60+
}
61+
62+
/**
63+
* @return whether the import starts with / or . indicating a relative import.
64+
* These would not be added to package.json dependencies.
65+
*/
66+
public boolean isRelative() {
67+
return from.startsWith("/") || from.startsWith(".");
68+
}
69+
70+
/**
71+
* @return whether the import should correspond to an entry in
72+
* package.json.
73+
*/
74+
public boolean isDeclarablePackageImport() {
75+
return !isNodejsNative() && !isRelative();
76+
}
77+
78+
/**
79+
* @return the package name. This excludes sub-paths of packages.
80+
*
81+
* For example in \@smithy/pkg/module the package name is \@smithy/pkg.
82+
*/
83+
public String getPackageName() {
84+
String[] packageNameSegments = from.split("/");
85+
String packageName;
86+
if (isNodejsNative()) {
87+
packageName = packageNameSegments[0].substring("node:".length());
88+
} else if (isNamespaced()) {
89+
packageName = packageNameSegments[0] + "/" + packageNameSegments[1];
90+
} else {
91+
packageName = packageNameSegments[0];
92+
}
93+
return packageName;
94+
}
95+
}

‎smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/TypeScriptWriterTest.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,17 @@ public void writesDocStrings() {
2121
public void doesNotAddNewlineBetweenManagedAndExplicitImports() {
2222
TypeScriptWriter writer = new TypeScriptWriter("foo");
2323
writer.write("import { Foo } from \"baz\";");
24-
writer.addImport("Baz", "Baz", "hello");
24+
writer.addImport("Baz", "Baz", "./hello");
2525
writer.addImport("Bar", "__Bar", TypeScriptDependency.SMITHY_TYPES);
2626
writer.addRelativeImport("Qux", "__Qux", Paths.get("./qux"));
2727
String result = writer.toString();
2828

29-
assertThat(result, equalTo(CODEGEN_INDICATOR + "import { Qux as __Qux } from \"./qux\";\n"
30-
+ "import { Bar as __Bar } from \"@smithy/types\";\n"
31-
+ "import { Baz } from \"hello\";\n"
32-
+ "import { Foo } from \"baz\";\n"));
29+
assertThat(result, equalTo("""
30+
%simport { Baz } from "./hello";
31+
import { Qux as __Qux } from "./qux";
32+
import { Bar as __Bar } from "@smithy/types";
33+
import { Foo } from "baz";
34+
""".formatted(CODEGEN_INDICATOR)));
3335
}
3436

3537
@Test

‎smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/validation/ImportFromTest.java

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package software.amazon.smithy.typescript.codegen.validation;
2+
3+
import org.junit.jupiter.api.Test;
4+
5+
import static org.junit.jupiter.api.Assertions.*;
6+
7+
class ImportFromTest {
8+
9+
@Test
10+
void isNodejsNative() {
11+
assertTrue(
12+
new ImportFrom("node:buffer").isNodejsNative()
13+
);
14+
assertTrue(
15+
new ImportFrom("stream").isNodejsNative()
16+
);
17+
assertFalse(
18+
new ImportFrom("@smithy/util").isNodejsNative()
19+
);
20+
assertFalse(
21+
new ImportFrom("../file").isNodejsNative()
22+
);
23+
}
24+
25+
@Test
26+
void isNamespaced() {
27+
assertTrue(
28+
new ImportFrom("@smithy/util/submodule").isNamespaced()
29+
);
30+
assertTrue(
31+
new ImportFrom("@smithy/util").isNamespaced()
32+
);
33+
assertFalse(
34+
new ImportFrom("node:stream").isNamespaced()
35+
);
36+
assertFalse(
37+
new ImportFrom("fs/promises").isNamespaced()
38+
);
39+
}
40+
41+
@Test
42+
void isRelative() {
43+
assertTrue(
44+
new ImportFrom("/file/path").isRelative()
45+
);
46+
assertTrue(
47+
new ImportFrom("./file/path").isRelative()
48+
);
49+
assertTrue(
50+
new ImportFrom("../../../../file/path").isRelative()
51+
);
52+
assertFalse(
53+
new ImportFrom("@smithy/util").isRelative()
54+
);
55+
assertFalse(
56+
new ImportFrom("fs/promises").isRelative()
57+
);
58+
}
59+
60+
@Test
61+
void isDeclarablePackageImport() {
62+
assertTrue(
63+
new ImportFrom("@smithy/util/submodule").isDeclarablePackageImport()
64+
);
65+
assertTrue(
66+
new ImportFrom("@smithy/util").isDeclarablePackageImport()
67+
);
68+
assertTrue(
69+
new ImportFrom("smithy_pkg").isDeclarablePackageImport()
70+
);
71+
assertTrue(
72+
new ImportFrom("smithy_pkg/array").isDeclarablePackageImport()
73+
);
74+
assertFalse(
75+
new ImportFrom("node:buffer").isDeclarablePackageImport()
76+
);
77+
assertFalse(
78+
new ImportFrom("../pkg/pkg").isDeclarablePackageImport()
79+
);
80+
}
81+
82+
@Test
83+
void getPackageName() {
84+
assertEquals(
85+
new ImportFrom("smithy_pkg/array").getPackageName(),
86+
"smithy_pkg"
87+
);
88+
assertEquals(
89+
new ImportFrom("@smithy/util/submodule").getPackageName(),
90+
"@smithy/util"
91+
);
92+
assertEquals(
93+
new ImportFrom("node:fs/promises").getPackageName(),
94+
"fs"
95+
);
96+
assertEquals(
97+
new ImportFrom("smithy_pkg").getPackageName(),
98+
"smithy_pkg"
99+
);
100+
}
101+
}

0 commit comments

Comments
 (0)