Merge branch 'master' into rmuller/py-license

CONTRIBUTING.md

@@ -11,20 +11,25 @@ Due to the polyglot nature of `jsii`, the toolchain requirements are somewhat
 more complicated than for most projects. In order to locally develop `jsii`, you
 will need the following tools:
 
-- [Node 8.11.0] or later
-- An OpenJDK-8 distribution
-  + [Oracle's OpenJDK8]
-  + [Amazon Corretto 8]
-- [.NET Core 2.0] or later
-- [Python 3.6.5] or later
-- [Ruby 2.5.1] or later
-
-[Node 8.11.0]: https://nodejs.org/download/release/v8.11.0/
+- [Node `8.11.0`] or later
+- An OpenJDK-8 distribution (e.g: [Oracle's OpenJDK8], [Amazon Corretto 8])
+  + [`maven >= 3.0.5`](https://maven.apache.org)
+- [.NET Core `2.0`] or later
+  + *Recommended:* [`mono >= 5`](https://www.mono-project.com)
+- [Python `3.6.5`] or later
+  + [`pip`](https://pip.pypa.io/en/stable/installing/)
+  + [`setuptools >= 38.6.0`](https://pypi.org/project/setuptools/)
+  + [`wheel`](https://pypi.org/project/wheel/)
+  + *Recommended:* [`twine`](https://pypi.org/project/twine/)
+- [Ruby `2.4.4p296`] or later
+  + [`bundler ~> 1.17.2`](https://bundler.io)
+
+[Node `8.11.0`]: https://nodejs.org/download/release/v8.11.0/
 [Oracle's OpenJDK8]: http://openjdk.java.net/install/
 [Amazon Corretto 8]: https://aws.amazon.com/corretto/
-[.NET Core 2.0]: https://www.microsoft.com/net/download
-[Python 3.6.5]: https://www.python.org/downloads/release/python-365/
-[Ruby 2.5.1]: https://www.ruby-lang.org/en/news/2018/03/28/ruby-2-5-1-released/
+[.NET Core `2.0`]: https://www.microsoft.com/net/download
+[Python `3.6.5`]: https://www.python.org/downloads/release/python-365/
+[Ruby `2.4.4p296`]: https://www.ruby-lang.org/en/news/2018/03/28/ruby-2-5-1-released/
 
 ### Alterative: build in Docker
 

packages/jsii-pacmak/lib/logging.ts

@@ -1,14 +1,20 @@
-export let level = 0;
-
-export const LEVEL_INFO    = 1;
-export const LEVEL_VERBOSE = 2;
-
 export enum Level {
+    WARN = -1,
     QUIET = 0,
     INFO = 1,
     VERBOSE = 2,
 }
 
+export const LEVEL_INFO: number = Level.INFO;
+export const LEVEL_VERBOSE: number = Level.VERBOSE;
+
+/** The minimal logging level for messages to be emitted. */
+export let level = Level.QUIET;
+
+export function warn(fmt: string, ...args: any[]) {
+    log(Level.WARN, fmt, ...args);
+}
+
 export function info(fmt: string, ...args: any[]) {
     log(Level.INFO, fmt, ...args);
 }
@@ -23,4 +29,4 @@ function log(messageLevel: Level, fmt: string, ...args: any[]) {
         // tslint:disable-next-line:no-console
         console.error.call(console, ...[ `[jsii-pacmak] [${levelName}]`, fmt, ...args ]);
     }
-}
+}

packages/jsii-pacmak/lib/targets/python.ts

@@ -6,6 +6,7 @@ import * as reflect from 'jsii-reflect';
 import * as spec from 'jsii-spec';
 import { Stability } from 'jsii-spec';
 import { Generator, GeneratorOptions } from '../generator';
+import { warn } from '../logging';
 import { md2rst } from '../markdown';
 import { propertySpec } from '../reflect-hacks';
 import { Target, TargetOptions } from '../target';
@@ -31,6 +32,22 @@ export default class Python extends Target {
         // Actually package up our code, both as a sdist and a wheel for publishing.
         await shell("python3", ["setup.py", "sdist", "--dist-dir", outDir], { cwd: sourceDir });
         await shell("python3", ["setup.py", "bdist_wheel", "--dist-dir", outDir], { cwd: sourceDir });
+        if (await twineIsPresent()) {
+            await shell("twine", ["check", path.join(outDir, '*')], { cwd: sourceDir });
+        } else {
+            warn('Unable to validate distribution packages because `twine` is not present. '
+                + 'Run `pip3 install twine` to enable distribution package validation.');
+        }
+
+        // Approximating existence check using `pip3 show`. If that fails, assume twine is not there.
+        async function twineIsPresent(): Promise<boolean> {
+            try {
+                const output = await shell("pip3", ["show", "twine"], { cwd: sourceDir });
+                return output.trim() !== '';
+            } catch {
+                return false;
+            }
+        }
     }
 }
 

packages/jsii/bin/jsii.ts

@@ -22,6 +22,11 @@ import { VERSION } from '../lib/version';
             default: true,
             desc: 'Automatically add missing entries in the peerDependencies section of package.json'
         })
+        .options('fail-on-warnings', {
+            alias: 'Werr',
+            type: 'boolean',
+            desc: 'Treat warnings as errors'
+        })
         .help()
         .version(VERSION)
         .argv;
@@ -35,15 +40,16 @@ import { VERSION } from '../lib/version';
     const compiler = new Compiler({
         projectInfo,
         watch: argv.watch,
-        projectReferences: argv['project-references']
+        projectReferences: argv['project-references'],
+        failOnWarnings: argv['fail-on-warnings']
     });
 
     return { projectRoot, emitResult: await compiler.emit() };
 })().then(({ projectRoot, emitResult }) => {
     for (const diagnostic of emitResult.diagnostics) {
         utils.logDiagnostic(diagnostic, projectRoot);
     }
-    if (emitResult.hasErrors) {
+    if (emitResult.emitSkipped) {
         process.exit(1);
     }
 }).catch(e => {

packages/jsii/lib/assembler.ts

@@ -12,6 +12,7 @@ import { getReferencedDocParams, parseSymbolDocumentation } from './docs';
 import { Diagnostic, EmitResult, Emitter } from './emitter';
 import literate = require('./literate');
 import { ProjectInfo } from './project-info';
+import { isReservedName } from './reserved-words';
 import { Validator } from './validator';
 import { SHORT_VERSION, VERSION } from './version';
 
@@ -91,7 +92,7 @@ export class Assembler implements Emitter {
       // Clearing ``this._types`` to allow contents to be garbage-collected.
       delete this._types;
       try {
-        return { diagnostics: this._diagnostics, hasErrors: true };
+        return { diagnostics: this._diagnostics, emitSkipped: true };
       } finally {
         // Clearing ``this._diagnostics`` to allow contents to be garbage-collected.
         delete this._diagnostics;
@@ -124,7 +125,7 @@ export class Assembler implements Emitter {
 
     const validator = new Validator(this.projectInfo, assembly);
     const validationResult = await validator.emit();
-    if (!validationResult.hasErrors) {
+    if (!validationResult.emitSkipped) {
       const assemblyPath = path.join(this.projectInfo.projectRoot, '.jsii');
       LOG.trace(`Emitting assembly: ${colors.blue(assemblyPath)}`);
       await fs.writeJson(assemblyPath, _fingerprint(assembly), { encoding: 'utf8', spaces: 2 });
@@ -133,7 +134,7 @@ export class Assembler implements Emitter {
     try {
       return {
         diagnostics: [...this._diagnostics, ...validationResult.diagnostics],
-        hasErrors: validationResult.hasErrors
+        emitSkipped: validationResult.emitSkipped
       };
     } finally {
       // Clearing ``this._types`` to allow contents to be garbage-collected.
@@ -439,6 +440,8 @@ export class Assembler implements Emitter {
       return undefined;
     }
 
+    this._warnAboutReservedWords(type.symbol);
+
     const fqn = `${[this.projectInfo.name, ...ctx.namespace].join('.')}.${type.symbol.name}`;
 
     const jsiiType: spec.ClassType = {
@@ -747,6 +750,8 @@ export class Assembler implements Emitter {
       return undefined;
     }
 
+    this._warnAboutReservedWords(type.symbol);
+
     const decl = symbol.valueDeclaration;
     const flags = ts.getCombinedModifierFlags(decl);
     // tslint:disable-next-line:no-bitwise
@@ -822,6 +827,8 @@ export class Assembler implements Emitter {
       return undefined;
     }
 
+    this._warnAboutReservedWords(type.symbol);
+
     const fqn = `${[this.projectInfo.name, ...ctx.namespace].join('.')}.${type.symbol.name}`;
 
     const jsiiType: spec.InterfaceType = {
@@ -954,6 +961,8 @@ export class Assembler implements Emitter {
       this._diagnostic(declaration, ts.DiagnosticCategory.Error, `Prohibited member name: ${symbol.name}`);
       return;
     }
+    this._warnAboutReservedWords(symbol);
+
     const parameters = await Promise.all(signature.getParameters().map(p => this._toParameter(p, ctx)));
 
     const returnType = signature.getReturnType();
@@ -1006,6 +1015,16 @@ export class Assembler implements Emitter {
     type.methods.push(method);
   }
 
+  private _warnAboutReservedWords(symbol: ts.Symbol) {
+    const reservingLanguages = isReservedName(symbol.name);
+    if (reservingLanguages) {
+      this._diagnostic(symbol.valueDeclaration,
+        ts.DiagnosticCategory.Warning,
+        `'${symbol.name}' is a reserved word in ${reservingLanguages.join(', ')}. Using this name may cause problems `
+        + 'when generating language bindings. Consider using a different name.');
+    }
+  }
+
   private async _visitProperty(symbol: ts.Symbol, type: spec.ClassType | spec.InterfaceType, ctx: EmitContext) {
     if (type.properties && type.properties.find(p => p.name === symbol.name)) {
       /*
@@ -1024,6 +1043,8 @@ export class Assembler implements Emitter {
       return;
     }
 
+    this._warnAboutReservedWords(symbol);
+
     const signature = symbol.valueDeclaration as (ts.PropertySignature
       | ts.PropertyDeclaration
       | ts.AccessorDeclaration
@@ -1069,6 +1090,8 @@ export class Assembler implements Emitter {
     }
     const paramDeclaration = paramSymbol.valueDeclaration as ts.ParameterDeclaration;
 
+    this._warnAboutReservedWords(paramSymbol);
+
     const parameter: spec.Parameter = {
       ...await this._optionalValue(this._typeChecker.getTypeAtLocation(paramSymbol.valueDeclaration), paramSymbol.valueDeclaration),
       name: paramSymbol.name,
@@ -1643,12 +1666,11 @@ function isErrorType(t: ts.Type) {
 }
 
 /**
- * These specifially cause trouble in C#, where we have to specificially annotate them as 'new' but our generator isn't doing that
- *
- * In C#, 'GetHashCode' is also problematic, but jsii already prevents you from naming a
- * method that starts with 'get' so we don't need to do anything special for that.
+ * Those have specific semantics in certain languages that don't always translate cleanly in others
+ * (like how equals/hashCode are not a thing in Javascript, but carry meaning in Java and C#). The
+ * `build` name is reserved for generated code (Java builders use that).
  */
-const PROHIBITED_MEMBER_NAMES = ['equals', 'hashcode'];
+const PROHIBITED_MEMBER_NAMES = ['build', 'equals', 'hashcode'];
 
 /**
  * Whether the given name is prohibited

packages/jsii/lib/compiler.ts

@@ -44,6 +44,8 @@ export interface CompilerOptions {
     watch?: boolean;
     /** Whether to detect and generate TypeScript project references */
     projectReferences?: boolean;
+    /** Whether to fail when a warning is emitted */
+    failOnWarnings?: boolean;
 }
 
 export interface TypescriptConfig {
@@ -147,7 +149,7 @@ export class Compiler implements Emitter {
 
     private async _consumeProgram(program: ts.Program, stdlib: string): Promise<EmitResult> {
         const emit = program.emit();
-        let hasErrors = emitHasErrors(emit);
+        let hasErrors = emitHasErrors(emit, this.options.failOnWarnings);
         const diagnostics = [...emit.diagnostics];
 
         if (hasErrors) {
@@ -160,17 +162,17 @@ export class Compiler implements Emitter {
         try {
             const assembler = new Assembler(this.options.projectInfo, program, stdlib);
             const assmEmit = await assembler.emit();
-            if (assmEmit.hasErrors) {
+            if (assmEmit.emitSkipped) {
                 LOG.error('Type model errors prevented the JSII assembly from being created');
             }
 
-            hasErrors = hasErrors || assmEmit.hasErrors;
+            hasErrors = hasErrors || emitHasErrors(assmEmit, this.options.failOnWarnings);
             diagnostics.push(...assmEmit.diagnostics);
         } catch (e) {
             LOG.error(`Error during type model analysis: ${e}`);
         }
 
-        return { hasErrors, diagnostics };
+        return { emitSkipped: hasErrors, diagnostics, emittedFiles: emit.emittedFiles };
     }
 
     /**
@@ -371,6 +373,9 @@ function parseConfigHostFromCompilerHost(host: ts.CompilerHost): ts.ParseConfigH
     };
 }
 
-function emitHasErrors(result: ts.EmitResult) {
-    return result.diagnostics.some(d => d.category === ts.DiagnosticCategory.Error) || result.emitSkipped;
+function emitHasErrors(result: ts.EmitResult, includeWarnings?: boolean) {
+    return result.diagnostics.some(d =>
+        d.category === ts.DiagnosticCategory.Error
+        || (includeWarnings && d.category === ts.DiagnosticCategory.Warning)
+    ) || result.emitSkipped;
 }

packages/jsii/lib/emitter.ts

@@ -15,10 +15,7 @@ export interface Emitter {
 /**
  * The result of attempting to emit stuff.
  */
-export interface EmitResult {
-    /** Whether the emit was skipped as a result of errors (found in ``diagnostics``) */
-    hasErrors: boolean;
-
+export interface EmitResult extends ts.EmitResult {
     /** Diagnostic information created when trying to emit stuff */
     diagnostics: ReadonlyArray<ts.Diagnostic | Diagnostic>;
 }