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

feat(pacmak): go module resolution & compiler fixes #1956

Merged
merged 1 commit into from
Aug 28, 2020

Conversation

MrArnoldPalmer
Copy link
Contributor

@MrArnoldPalmer MrArnoldPalmer commented Aug 27, 2020

Fixes a number of compiler bugs and adds support for external package
dependency resolution.

Changes package file layout to prevent extraneous directory nesting.

Adds logic to RootPackage to be able to resolve types within external
jsii modules. Also adds a moduleName field to the go target jsii
config to allow imports across packages to resolve correctly.

Changes logic of embedded interfaces to correctly resolve using local
names and bubble embedded interfaces up to the package dependencies
array to make sure their imports are resolved.

Adds utility function for substituting reserved words in
variable/function argument names. Some examples within jsii-calc have
arguments named map which caused go compiler errors. More reserved
words should be added to a central list for this functionality in the
future.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@MrArnoldPalmer MrArnoldPalmer added this to the GoLang Alpha milestone Aug 27, 2020
@mergify
Copy link
Contributor

mergify bot commented Aug 27, 2020

The title of this Pull Request does not conform with [Conventional Commits] guidelines. It will need to be adjusted before the PR can be merged.
[Conventional Commits]: https://www.conventionalcommits.org

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Aug 27, 2020
@MrArnoldPalmer MrArnoldPalmer changed the title feat(pacmak) - module resolution & compiler fixes feat(pacmak): module resolution & compiler fixes Aug 27, 2020
@SoManyHs SoManyHs self-assigned this Aug 27, 2020
@MrArnoldPalmer MrArnoldPalmer changed the title feat(pacmak): module resolution & compiler fixes feat(pacmak): go module resolution & compiler fixes Aug 27, 2020
packages/jsii-pacmak/lib/targets/golang/types/class.ts Outdated Show resolved Hide resolved
packages/jsii-pacmak/lib/targets/golang/types/class.ts Outdated Show resolved Hide resolved
packages/jsii-pacmak/lib/targets/golang/types/class.ts Outdated Show resolved Hide resolved
packages/jsii-pacmak/lib/targets/golang/types/class.ts Outdated Show resolved Hide resolved
import { TypeField } from './type-field';
import { getFieldDependencies } from '../util';

class InterfaceProperty implements TypeField {
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this go type should only refer to a behavioral interface, there shouldn't be any properties on this, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Behavioral interfaces have properties and methods. DataType interfaces have only properties. We could merge the logic of these two. This class only serves to replace the inline emit of property getters/setters in Interface so that we can resolve dependencies of each one.

Comment on lines 24 to 31
public emit(code: CodeMaker) {
const propName = code.toPascalCase(this.name);
const type = new GoTypeRef(
this.parent.parent.root,
this.property.type,
).scopedName(this.parent.parent);

code.line(`Get${propName}() ${type}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interface doesn't currently inherit from from struct. We can/should do that for property shared property emit logic but I'd say thats another PR.

Fixes a number of compiler bugs and adds support for external package
dependency resolution.

Changes package file layout to prevent extraneous directory nesting.

Adds logic to `RootPackage` to be able to resolve types within external
jsii modules. Also adds a `moduleName` field to the go target jsii
config to allow imports across packages to resolve correctly.

Changes logic of embedded interfaces to correctly resolve using local
names and bubble embedded interfaces up to the package dependencies
array to make sure their imports are resolved.

Adds utility function for substituting reserved words in
variable/function argument names. Some examples within jsii-calc have
arguments named `map` which caused go compiler errors. More reserved
words should be added to a central list for this functionality in the
future.
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-Blkkw9bQFn8A
  • Commit ID: 11a04c9
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@MrArnoldPalmer MrArnoldPalmer merged commit 7e2512f into aws:master Aug 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants