Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: check dependencies when adding imports #1239

Merged
merged 9 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.util.function.UnaryOperator;
import software.amazon.smithy.codegen.core.CodegenException;
import software.amazon.smithy.codegen.core.Symbol;
import software.amazon.smithy.codegen.core.SymbolDependency;
import software.amazon.smithy.codegen.core.SymbolReference;
import software.amazon.smithy.codegen.core.SymbolWriter;
import software.amazon.smithy.model.Model;
Expand All @@ -29,6 +30,7 @@
import software.amazon.smithy.model.traits.DeprecatedTrait;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.InternalTrait;
import software.amazon.smithy.typescript.codegen.validation.ImportFrom;
import software.amazon.smithy.utils.SmithyUnstableApi;
import software.amazon.smithy.utils.StringUtils;

Expand Down Expand Up @@ -114,19 +116,40 @@ public TypeScriptWriter addIgnoredDefaultImport(String name, String from, String
*/
@Deprecated
public TypeScriptWriter addImport(String name, String as, String from) {
ImportFrom importFrom = new ImportFrom(from);

if (importFrom.isDeclarablePackageImport()) {
String packageName = importFrom.getPackageName();
if (getDependencies()
.stream()
.map(SymbolDependency::getPackageName)
.noneMatch(packageName::equals)) {
throw new CodegenException(
"""
The import %s does not correspond to a registered dependency.
TypeScriptWriter::addDependency() is required before ::addImport().
""".formatted(from)
);
}
}

getImportContainer().addImport(name, as, from);
return this;
}

/**
* Imports a type using an alias from a module only if necessary.
* Adds the dependency.
*
* @param name Type to import.
* @param as Alias to refer to the type as.
* @param from PackageContainer to import the type from.
* @return Returns the writer.
*/
public TypeScriptWriter addImport(String name, String as, PackageContainer from) {
if (from instanceof Dependency) {
addDependency((Dependency) from);
}
Comment on lines 149 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

In the long run, we should move it to taking Dependency as the third parameter.

return this.addImport(name, as, from.getPackageName());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

package software.amazon.smithy.typescript.codegen.validation;

import java.util.Set;
import software.amazon.smithy.utils.SetUtils;
import software.amazon.smithy.utils.SmithyInternalApi;

/**
* Interprets the string portion of an import statement.
*/
@SmithyInternalApi
public class ImportFrom {
kuhe marked this conversation as resolved.
Show resolved Hide resolved
public static final Set<String> NODE_NATIVE_DEPENDENCIES = SetUtils.of(
"buffer",
"child_process",
"crypto",
"dns",
"events",
"fs",
"http",
"http2",
"https",
"os",
"path",
"process",
"stream",
"tls",
"url",
"util",
"zlib"
);

private final String from;

public ImportFrom(String importTargetExpression) {
this.from = importTargetExpression;
}

/**
* @return whether we recognize it as a Node.js native module. These
* do not need to be declared in package.json. This check
* is not exhaustive since the list of native modules varies
* by version.
*/
public boolean isNodejsNative() {
String[] packageNameSegments = from.split("/");
return from.startsWith("node:")
|| NODE_NATIVE_DEPENDENCIES.contains(packageNameSegments[0]);
}

/**
* @return whether the import has an org or namespace prefix like \@smithy/pkg.
*/
public boolean isNamespaced() {
return from.startsWith("@") && from.contains("/");
}

/**
* @return whether the import starts with / or . indicating a relative import.
* These would not be added to package.json dependencies.
*/
public boolean isRelative() {
return from.startsWith("/") || from.startsWith(".");
}

/**
* @return whether the import should correspond to an entry in
* package.json.
*/
public boolean isDeclarablePackageImport() {
return !isNodejsNative() && !isRelative();
}

/**
* @return the package name. This excludes sub-paths of packages.
*
* For example in \@smithy/pkg/module the package name is \@smithy/pkg.
*/
public String getPackageName() {
String[] packageNameSegments = from.split("/");
String packageName;
if (isNodejsNative()) {
packageName = packageNameSegments[0].substring("node:".length());
} else if (isNamespaced()) {
packageName = packageNameSegments[0] + "/" + packageNameSegments[1];
} else {
packageName = packageNameSegments[0];
}
return packageName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@ public void writesDocStrings() {
public void doesNotAddNewlineBetweenManagedAndExplicitImports() {
TypeScriptWriter writer = new TypeScriptWriter("foo");
writer.write("import { Foo } from \"baz\";");
writer.addImport("Baz", "Baz", "hello");
writer.addImport("Baz", "Baz", "./hello");
writer.addImport("Bar", "__Bar", TypeScriptDependency.SMITHY_TYPES);
writer.addRelativeImport("Qux", "__Qux", Paths.get("./qux"));
String result = writer.toString();

assertThat(result, equalTo(CODEGEN_INDICATOR + "import { Qux as __Qux } from \"./qux\";\n"
+ "import { Bar as __Bar } from \"@smithy/types\";\n"
+ "import { Baz } from \"hello\";\n"
+ "import { Foo } from \"baz\";\n"));
assertThat(result, equalTo("""
%simport { Baz } from "./hello";
import { Qux as __Qux } from "./qux";
import { Bar as __Bar } from "@smithy/types";
import { Foo } from "baz";
""".formatted(CODEGEN_INDICATOR)));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package software.amazon.smithy.typescript.codegen.validation;

import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.*;

class ImportFromTest {

@Test
void isNodejsNative() {
assertTrue(
new ImportFrom("node:buffer").isNodejsNative()
);
assertTrue(
new ImportFrom("stream").isNodejsNative()
);
assertFalse(
new ImportFrom("@smithy/util").isNodejsNative()
);
assertFalse(
new ImportFrom("../file").isNodejsNative()
);
}

@Test
void isNamespaced() {
assertTrue(
new ImportFrom("@smithy/util/submodule").isNamespaced()
);
assertTrue(
new ImportFrom("@smithy/util").isNamespaced()
);
assertFalse(
new ImportFrom("node:stream").isNamespaced()
);
assertFalse(
new ImportFrom("fs/promises").isNamespaced()
);
}

@Test
void isRelative() {
assertTrue(
new ImportFrom("/file/path").isRelative()
);
assertTrue(
new ImportFrom("./file/path").isRelative()
);
assertTrue(
new ImportFrom("../../../../file/path").isRelative()
);
assertFalse(
new ImportFrom("@smithy/util").isRelative()
);
assertFalse(
new ImportFrom("fs/promises").isRelative()
);
}

@Test
void isDeclarablePackageImport() {
assertTrue(
new ImportFrom("@smithy/util/submodule").isDeclarablePackageImport()
);
assertTrue(
new ImportFrom("@smithy/util").isDeclarablePackageImport()
);
assertTrue(
new ImportFrom("smithy_pkg").isDeclarablePackageImport()
);
assertTrue(
new ImportFrom("smithy_pkg/array").isDeclarablePackageImport()
);
assertFalse(
new ImportFrom("node:buffer").isDeclarablePackageImport()
);
assertFalse(
new ImportFrom("../pkg/pkg").isDeclarablePackageImport()
);
}

@Test
void getPackageName() {
assertEquals(
new ImportFrom("smithy_pkg/array").getPackageName(),
"smithy_pkg"
);
assertEquals(
new ImportFrom("@smithy/util/submodule").getPackageName(),
"@smithy/util"
);
assertEquals(
new ImportFrom("node:fs/promises").getPackageName(),
"fs"
);
assertEquals(
new ImportFrom("smithy_pkg").getPackageName(),
"smithy_pkg"
);
}
}
Loading