Skip to content

Commit

Permalink
fix: #125 - Inserting a namespace or class into an ambient module/nam…
Browse files Browse the repository at this point in the history
…espace should not write as non-ambient.
  • Loading branch information
dsherret committed Apr 17, 2018
1 parent f027ea8 commit 297955b
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 23 deletions.
2 changes: 1 addition & 1 deletion docs/details/comment-ranges.md
Expand Up @@ -12,7 +12,7 @@ Comment ranges are not part of the AST and are generated on request.
WARNING: Since comments are generated on demand and not part of the AST, using one after a
subsequent manipulation to the source file will throw an error.

### Retreiving
### Retrieving

Leading and trailing comment ranges can be retrieved from any node by calling:

Expand Down
17 changes: 2 additions & 15 deletions src/compiler/base/AmbientableNode.ts
Expand Up @@ -2,7 +2,7 @@ import { ts, SyntaxKind } from "../../typescript";
import { Constructor } from "../../Constructor";
import * as errors from "../../errors";
import { AmbientableNodeStructure } from "../../structures";
import { TypeGuards } from "../../utils";
import { TypeGuards, isNodeAmbientOrInAmbientContext } from "../../utils";
import { callBaseFill } from "../callBaseFill";
import { Node } from "../common";
import { ModifierableNode } from "./ModifierableNode";
Expand Down Expand Up @@ -48,20 +48,7 @@ export function AmbientableNode<T extends Constructor<AmbientableNodeExtensionTy
}

isAmbient() {
const isThisAmbient = (this.getCombinedModifierFlags() & ts.ModifierFlags.Ambient) === ts.ModifierFlags.Ambient;
if (isThisAmbient || TypeGuards.isInterfaceDeclaration(this) || TypeGuards.isTypeAliasDeclaration(this))
return true;

let topParent = this as Node;
for (const parent of this.getAncestors()) {
topParent = parent; // store the top parent for later

const modifierFlags = parent.getCombinedModifierFlags();
if (modifierFlags & ts.ModifierFlags.Ambient)
return true;
}

return TypeGuards.isSourceFile(topParent) && topParent.isDeclarationFile();
return isNodeAmbientOrInAmbientContext(this);
}

setHasDeclareKeyword(value: boolean) {
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/statement/StatementedNode.ts
Expand Up @@ -6,7 +6,8 @@ import { ClassDeclarationStructure, InterfaceDeclarationStructure, TypeAliasDecl
EnumDeclarationStructure, NamespaceDeclarationStructure, StatementedNodeStructure, VariableStatementStructure } from "../../structures";
import { verifyAndGetIndex, insertIntoBracesOrSourceFileWithGetChildren, getRangeFromArray, removeStatementedNodeChildren,
hasBody, InsertIntoBracesOrSourceFileOptionsWriteInfo } from "../../manipulation";
import { getNodeByNameOrFindFunction, getNotFoundErrorMessageForNameOrFindFunction, TypeGuards, ArrayUtils, getSyntaxKindName } from "../../utils";
import { getNodeByNameOrFindFunction, getNotFoundErrorMessageForNameOrFindFunction, TypeGuards, ArrayUtils, getSyntaxKindName,
isNodeAmbientOrInAmbientContext } from "../../utils";
import { callBaseFill } from "../callBaseFill";
import { Node } from "../common";
import { SourceFile } from "../file";
Expand Down Expand Up @@ -510,7 +511,7 @@ export function StatementedNode<T extends Constructor<StatementedNodeExtensionTy
structures,
write: (writer, info) => {
this._standardWrite(writer, info, () => {
this.global.structurePrinterFactory.forClassDeclaration({ isAmbient: this.sourceFile.isDeclarationFile() }).printTexts(writer, structures);
this.global.structurePrinterFactory.forClassDeclaration({ isAmbient: isNodeAmbientOrInAmbientContext(this) }).printTexts(writer, structures);
});
}
});
Expand Down Expand Up @@ -696,7 +697,7 @@ export function StatementedNode<T extends Constructor<StatementedNodeExtensionTy
structures,
write: (writer, info) => {
this._standardWrite(writer, info, () => {
this.global.structurePrinterFactory.forNamespaceDeclaration({ isAmbient: this.sourceFile.isDeclarationFile() }).printTexts(writer, structures);
this.global.structurePrinterFactory.forNamespaceDeclaration({ isAmbient: isNodeAmbientOrInAmbientContext(this) }).printTexts(writer, structures);
});
}
});
Expand Down
13 changes: 9 additions & 4 deletions src/tests/compiler/class/classDeclarationTests.ts
Expand Up @@ -3,8 +3,9 @@ import { ClassDeclaration, MethodDeclaration, PropertyDeclaration, GetAccessorDe
ConstructorDeclaration, ParameterDeclaration, Scope } from "../../../compiler";
import { PropertyDeclarationStructure, MethodDeclarationStructure, ConstructorDeclarationStructure, ClassDeclarationSpecificStructure,
GetAccessorDeclarationStructure, SetAccessorDeclarationStructure } from "../../../structures";
import { getInfoFromText } from "../testHelpers";
import { SyntaxKind } from "../../../typescript";
import { TypeGuards } from "../../../utils";
import { getInfoFromText, getInfoFromTextWithDescendant } from "../testHelpers";

describe(nameof(ClassDeclaration), () => {
describe(nameof<ClassDeclaration>(c => c.fill), () => {
Expand Down Expand Up @@ -698,9 +699,9 @@ describe(nameof(ClassDeclaration), () => {

describe(nameof<ClassDeclaration>(d => d.insertMethods), () => {
function doTest(startCode: string, insertIndex: number, structures: MethodDeclarationStructure[], expectedCode: string) {
const {firstChild} = getInfoFromText<ClassDeclaration>(startCode);
const result = firstChild.insertMethods(insertIndex, structures);
expect(firstChild.getText()).to.equal(expectedCode);
const {descendant, sourceFile} = getInfoFromTextWithDescendant<ClassDeclaration>(startCode, SyntaxKind.ClassDeclaration);
const result = descendant.insertMethods(insertIndex, structures);
expect(sourceFile.getText()).to.equal(expectedCode);
expect(result.length).to.equal(structures.length);
}

Expand Down Expand Up @@ -743,6 +744,10 @@ describe(nameof(ClassDeclaration), () => {
" console.log('here');\n" +
" }\n}");
});

it("should write as ambient when inserting into a insert when none exists", () => {
doTest("declare module Ambient { class c {\n} }", 0, [{ name: "method" }], "declare module Ambient { class c {\n method();\n} }");
});
});

describe(nameof<ClassDeclaration>(d => d.insertMethod), () => {
Expand Down
11 changes: 11 additions & 0 deletions src/tests/compiler/statement/statementedNode/classTests.ts
Expand Up @@ -75,6 +75,17 @@ describe(nameof(StatementedNode), () => {
"}\n";
doTest("", 0, [structure], expectedText);
});

it("should insert an ambient method into a class when inserting a class into an ambient module", () => {
const {sourceFile} = getInfoFromText("declare module Namespace {\n}\n");
const namespaceDec = sourceFile.getNamespaces()[0];
namespaceDec.insertClasses(0, [{
name: "Identifier",
methods: [{ name: "myMethod" }]
}]);

expect(sourceFile.getFullText()).to.equal("declare module Namespace {\n class Identifier {\n myMethod();\n }\n}\n");
});
});

describe(nameof<StatementedNode>(n => n.insertClass), () => {
Expand Down
15 changes: 15 additions & 0 deletions src/tests/compiler/statement/statementedNode/namespaceTests.ts
Expand Up @@ -71,6 +71,21 @@ describe(nameof(StatementedNode), () => {
" console.log('here');\n" +
"}\n");
});

it("should insert an ambient method on a class class when inserting a namespace with a class into an ambient module", () => {
const { sourceFile } = getInfoFromText("declare module Namespace {\n}\n");
const namespaceDec = sourceFile.getNamespaces()[0];
namespaceDec.insertNamespaces(0, [{
name: "Namespace",
classes: [{
name: "Identifier",
methods: [{ name: "myMethod" }]
}]
}]);

expect(sourceFile.getFullText()).to.equal("declare module Namespace {\n namespace Namespace {\n class Identifier {\n" +
" myMethod();\n }\n }\n}\n");
});
});

describe(nameof<StatementedNode>(n => n.insertNamespace), () => {
Expand Down
1 change: 1 addition & 0 deletions src/utils/compiler/index.ts
Expand Up @@ -3,6 +3,7 @@ export * from "./getNodeByNameOrFindFunction";
export * from "./getParentSyntaxList";
export * from "./getSymbolByNameOrFindFunction";
export * from "./getSyntaxKindName";
export * from "./isNodeAmbientOrInAmbientContext";
export * from "./isStringKind";
export * from "./ModuleUtils";
export * from "./printNode";
20 changes: 20 additions & 0 deletions src/utils/compiler/isNodeAmbientOrInAmbientContext.ts
@@ -0,0 +1,20 @@
import { Node } from "../../compiler";
import { ts } from "../../typescript";
import { TypeGuards } from "../TypeGuards";

export function isNodeAmbientOrInAmbientContext(node: Node) {
if (checkNodeIsAmbient(node) || node.sourceFile.isDeclarationFile())
return true;

for (const ancestor of node.getAncestorsIterator(false)) {
if (checkNodeIsAmbient(ancestor))
return true;
}

return false;
}

function checkNodeIsAmbient(node: Node) {
const isThisAmbient = (node.getCombinedModifierFlags() & ts.ModifierFlags.Ambient) === ts.ModifierFlags.Ambient;
return isThisAmbient || TypeGuards.isInterfaceDeclaration(node) || TypeGuards.isTypeAliasDeclaration(node);
}

0 comments on commit 297955b

Please sign in to comment.