Auto merge pull request #568 from atomist/sdm

* Inspections now get push

* Introduce PushAwareParametersInvocation

* Add tests for push handling

* Autofix: TypeScript header

[atomist:generated] [atomist:autofix=typescript header]

* Autofix: tslint

[atomist:generated] [atomist:autofix=tslint]

* Autofix: TypeScript imports

[atomist:generated] [atomist:autofix=typescript imports]

* Remove considerOnlyChangedFiles option, which had broken semantics

CHANGELOG.md

@@ -42,6 +42,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 -   **BREAKING** Remove SDM-level goal methods. [#545](https://github.com/atomist/sdm/issues/545)
 -   **BREAKING** Remove old style registration methods. [#546](https://github.com/atomist/sdm/issues/546)
 -   **BREAKING** Remove client exports from SDM. [#547](https://github.com/atomist/sdm/issues/547)
+-   **BREAKING** Remove `considerOnlyChangedFiles` setting on autofix and inspection registrations. This should be handled
+by specific registrations.
 
 ### Fixed
 

lib/api-helper/listener/executeAutoInspects.ts

@@ -28,6 +28,7 @@ import {
 } from "../../api/goal/GoalInvocation";
 import { ParametersInvocation } from "../../api/listener/ParametersInvocation";
 import { AutoInspectRegistration } from "../../api/registration/AutoInspectRegistration";
+import { PushAwareParametersInvocation } from "../../api/registration/PushAwareParametersInvocation";
 import { PushImpactResponse } from "../../api/registration/PushImpactListenerRegistration";
 import {
     formatReviewerError,
@@ -130,8 +131,12 @@ function applyCodeInspections(goalInvocation: GoalInvocation,
             await Promise.all(relevantAutoInspects
                 .map(async autoInspect => {
                     const cli: ParametersInvocation<any> = createParametersInvocation(goalInvocation, autoInspect);
+                    const papi: PushAwareParametersInvocation<any> = {
+                        ...cli,
+                        push: cri,
+                    };
                     try {
-                        const inspectionResult = await autoInspect.inspection(project, cli);
+                        const inspectionResult = await autoInspect.inspection(project, papi);
                         const review = isProjectReview(inspectionResult) ? inspectionResult : undefined;
                         const response = autoInspect.onInspectionResult &&
                             await autoInspect.onInspectionResult(inspectionResult, cli).catch(err => undefined); // ignore errors

lib/api-helper/listener/executeAutofixes.ts

@@ -15,6 +15,7 @@
  */
 
 import {
+    HandlerContext,
     logger,
     RemoteRepoRef,
     Success,
@@ -31,6 +32,7 @@ import {
 import { ReportProgress } from "../../api/goal/progress/ReportProgress";
 import { PushImpactListenerInvocation } from "../../api/listener/PushImpactListener";
 import { AutofixRegistration } from "../../api/registration/AutofixRegistration";
+import { PushAwareParametersInvocation } from "../../api/registration/PushAwareParametersInvocation";
 import { ProgressLog } from "../../spi/log/ProgressLog";
 import { SdmGoalState } from "../../typings/types";
 import { confirmEditedness } from "../command/transform/confirmEditedness";
@@ -60,12 +62,12 @@ export function executeAutofixes(registrations: AutofixRegistration[]): ExecuteG
             const push = sdmGoal.push;
             const appliedAutofixes: AutofixRegistration[] = [];
             const editResult = await configuration.sdm.projectLoader.doWithProject<EditResult>({
-                credentials,
-                id,
-                context,
-                readOnly: false,
-                cloneOptions: minimalClone(push),
-            },
+                    credentials,
+                    id,
+                    context,
+                    readOnly: false,
+                    cloneOptions: minimalClone(push),
+                },
                 async project => {
                     if ((await project.gitStatus()).sha !== id.sha) {
                         return {
@@ -141,11 +143,16 @@ async function runOne(cri: PushImpactListenerInvocation,
                       autofix: AutofixRegistration,
                       progressLog: ProgressLog): Promise<EditResult> {
     const project = cri.project;
-    progressLog.write(sprintf("About to edit %s with autofix '%s'", (project.id as RemoteRepoRef).url, autofix.name));
+    progressLog.write(sprintf("About to transform %s with autofix '%s'", (project.id as RemoteRepoRef).url, autofix.name));
     try {
+        const arg2: HandlerContext & PushAwareParametersInvocation<any> = {
+            ...cri.context,
+            ...cri,
+            push: cri,
+        } as any;
         const tentativeEditResult = await toScalarProjectEditor(autofix.transform)(
             project,
-            cri.context,
+            arg2,
             autofix.parametersInstance);
         const editResult = await confirmEditedness(tentativeEditResult);
 

lib/api-helper/machine/handlerRegistrations.ts

@@ -53,6 +53,7 @@ import { isProject } from "@atomist/automation-client/lib/project/Project";
 import { toFactory } from "@atomist/automation-client/lib/util/constructionUtils";
 import { GitHubRepoTargets } from "../../api/command/target/GitHubRepoTargets";
 import { isTransformModeSuggestion } from "../../api/command/target/TransformModeSuggestion";
+import { SdmContext } from "../../api/context/SdmContext";
 import { CommandListenerInvocation } from "../../api/listener/CommandListener";
 import {
     isValidationError,
@@ -286,7 +287,8 @@ function toOnCommand<PARAMS>(c: CommandHandlerRegistration<any>): (sdm: MachineO
 }
 
 function toCommandListenerInvocation<P>(c: CommandRegistration<P>, context: HandlerContext, parameters: P): CommandListenerInvocation {
-    let credentials; // opts.credentialsResolver.commandHandlerCredentials(context, undefined);
+    // It may already be there
+    let credentials = !!context ? (context as any as SdmContext).credentials : undefined;
     let ids: RemoteRepoRef[];
     if (isSeedDrivenGeneratorParameters(parameters)) {
         credentials = parameters.target.credentials;

lib/api/registration/AutoInspectRegistration.ts

@@ -19,19 +19,14 @@ import { ParametersInvocation } from "../listener/ParametersInvocation";
 import { CodeInspection } from "./CodeInspectionRegistration";
 import {
     PushImpactResponse,
-    SelectiveCodeActionOptions,
 } from "./PushImpactListenerRegistration";
 import { PushSelector } from "./PushRegistration";
 
-export type AutoInspectRegistrationOptions = SelectiveCodeActionOptions;
-
 /**
  * Register an automatic inspection.
  */
 export interface AutoInspectRegistration<R, PARAMS = NoParameters> extends PushSelector {
 
-    options?: AutoInspectRegistrationOptions;
-
     /**
      * Inspection function to run on each project
      */

lib/api/registration/AutofixRegistration.ts

@@ -16,10 +16,9 @@
 
 import { NoParameters } from "@atomist/automation-client";
 import { CodeTransformOrTransforms } from "./CodeTransform";
-import { SelectiveCodeActionOptions } from "./PushImpactListenerRegistration";
 import { PushSelector } from "./PushRegistration";
 
-export interface AutofixRegistrationOptions extends SelectiveCodeActionOptions {
+export interface AutofixRegistrationOptions {
 
     ignoreFailure: boolean;
 }

lib/api/registration/CodeInspectionRegistration.ts

@@ -20,15 +20,15 @@ import {
     RepoRef,
 } from "@atomist/automation-client";
 import { CommandListenerInvocation } from "../listener/CommandListener";
-import { ParametersInvocation } from "../listener/ParametersInvocation";
 import { ProjectsOperationRegistration } from "./ProjectsOperationRegistration";
+import { PushAwareParametersInvocation } from "./PushAwareParametersInvocation";
 
 /**
  * Function that can run against a project without mutating it to
  * compute a value.
  */
 export type CodeInspection<R, P = NoParameters> = (p: Project,
-                                                   cli: ParametersInvocation<P>) => Promise<R>;
+                                                   papi: PushAwareParametersInvocation<P>) => Promise<R>;
 
 /**
  * Result of inspecting a single project

lib/api/registration/CodeTransform.ts

@@ -19,7 +19,7 @@ import {
     Project,
 } from "@atomist/automation-client";
 import { EditResult } from "@atomist/automation-client/lib/operations/edit/projectEditor";
-import { ParametersInvocation } from "../listener/ParametersInvocation";
+import { PushAwareParametersInvocation } from "./PushAwareParametersInvocation";
 
 export type TransformResult = EditResult;
 
@@ -32,15 +32,15 @@ export type TransformReturnable = Project | TransformResult | void;
  * Projects are naturally mutable.
  */
 export type CodeTransform<P = NoParameters> = (p: Project,
-                                               sdmc: ParametersInvocation<P>,
+                                               papi: PushAwareParametersInvocation<P>,
                                                params?: P) => Promise<TransformReturnable>;
 
 /**
  * Compatible with CodeTransform but returns TransformResult.
  * At the cost of greater ceremony, guarantees the return of more information.
  */
 export type ExplicitCodeTransform<P = NoParameters> = (p: Project,
-                                                       sdmc: ParametersInvocation<P>,
+                                                       papi: PushAwareParametersInvocation<P>,
                                                        params?: P) => Promise<TransformResult>;
 
 /**

lib/api/registration/PushAwareParametersInvocation.ts

@@ -0,0 +1,30 @@
+/*
+ * Copyright © 2018 Atomist, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { ParametersInvocation } from "../listener/ParametersInvocation";
+import { PushImpactListenerInvocation } from "../listener/PushImpactListener";
+
+/**
+ * Code inspections or autofixes may be invoked in response to a push,
+ * or just with Parameters
+ */
+export interface PushAwareParametersInvocation<P> extends ParametersInvocation<P> {
+
+    /**
+     * The push invocation. Will be undefined if we are not invoked from a push.
+     */
+    push?: PushImpactListenerInvocation;
+}

lib/api/registration/PushImpactListenerRegistration.ts

@@ -88,14 +88,3 @@ export function toPushReactionRegistration(prr: PushImpactListenerRegisterable<a
         action: prr,
     };
 }
-
-/**
- * Base options object for registrations that process selective files
- */
-export interface SelectiveCodeActionOptions {
-
-    /**
-     * Run only on affected files?
-     */
-    considerOnlyChangedFiles: boolean;
-}

test/api-helper/listener/executeAutoInspect.test.ts

@@ -33,6 +33,7 @@ import {
     ReviewListener,
     ReviewListenerInvocation,
 } from "../../../lib/api/listener/ReviewListener";
+import { AutoInspectRegistration } from "../../../lib/api/registration/AutoInspectRegistration";
 import { PushImpactResponse } from "../../../lib/api/registration/PushImpactListenerRegistration";
 
 const HatesTheWorld: ReviewerRegistration = {
@@ -49,7 +50,6 @@ const HatesTheWorld: ReviewerRegistration = {
                     offset: -1,
                 })),
     }),
-    options: { considerOnlyChangedFiles: false },
 };
 
 const JustTheOne: ReviewerRegistration = {
@@ -66,7 +66,6 @@ const JustTheOne: ReviewerRegistration = {
                     offset: -1,
                 })],
     }),
-    options: { considerOnlyChangedFiles: false },
 };
 
 function loggingReviewListenerWithApproval(saveTo: ReviewListenerInvocation[]): ReviewListener {
@@ -124,6 +123,31 @@ describe("executeAutoInspects", () => {
         assert(r.state);
     });
 
+    it("should find push", async () => {
+        const id = new GitHubRepoRef("a", "b");
+        const p = InMemoryProject.from(id, new InMemoryProjectFile("thing", "1"));
+        const rwlc = fakeGoalInvocation(id, {
+            projectLoader: new SingleProjectLoader(p),
+        } as any);
+        const errors: string[] = [];
+        const reg: AutoInspectRegistration<void> = {
+            name: "reg",
+            inspection: async (project, ci) => {
+                if (project !== p) {
+                    errors.push("Project not the same");
+                }
+                if (!ci.push || ci.push.push !== rwlc.sdmGoal.push) {
+                    errors.push("push should not be set");
+                }
+                assert(!ci);
+            },
+        };
+        const ge = executeAutoInspects([reg], []);
+
+        await ge(rwlc);
+        assert.deepEqual(errors, []);
+    });
+
     it("should hate anything it finds, without requiring approval", async () => {
         const id = new GitHubRepoRef("a", "b");
         const p = InMemoryProject.from(id, new InMemoryProjectFile("thing", "1"));

test/api-helper/listener/executeAutofixes.test.ts

@@ -52,6 +52,8 @@ export const AddThingAutofix: AutofixRegistration = {
     transform: async (project, ci) => {
         await project.addFile("thing", "1");
         assert(!!ci.context.workspaceId);
+        assert(project === ci.push.project);
+        assert(!!ci.credentials);
         assert(!ci.parameters);
         return { edited: true, success: true, target: project };
     },
@@ -158,10 +160,12 @@ describe("executeAutofixes", () => {
         (p as any as GitProject).push = async () => null;
         (p as any as GitProject).gitStatus = async () => ({ isClean: false, sha: "ec7fe33f7ee33eee84b3953def258d4e7ccb6783" } as any);
         const pl = new SingleProjectLoader(p);
-        const r = await executeAutofixes([AddThingAutofix])(fakeGoalInvocation(id, {
+        const gi = fakeGoalInvocation(id, {
             projectLoader: pl,
             repoRefResolver: FakeRepoRefResolver,
-        } as any)) as ExecuteGoalResult;
+        } as any);
+        assert(!!gi.credentials);
+        const r = await executeAutofixes([AddThingAutofix])(gi) as ExecuteGoalResult;
         assert.equal(r.code, 0);
         assert(!!p);
         const foundFile = p.findFileSync("thing");
@@ -190,6 +194,36 @@ describe("executeAutofixes", () => {
         assert.equal(foundFile.getContentSync(), "ibis");
     }).timeout(10000);
 
+    it("should execute with parameter and find push", async () => {
+        const id = GitHubRepoRef.from({ owner: "a", repo: "b", sha: "ec7fe33f7ee33eee84b3953def258d4e7ccb6783" });
+        const initialContent = "public class Thing {}";
+        const f = new InMemoryProjectFile("src/Thing.ts", initialContent);
+        const p = InMemoryProject.from(id, f, { path: "LICENSE", content: "Apache License" });
+        (p as any as GitProject).revert = async () => null;
+        (p as any as GitProject).commit = async () => null;
+        (p as any as GitProject).push = async () => null;
+        (p as any as GitProject).gitStatus = async () => ({ isClean: false, sha: "ec7fe33f7ee33eee84b3953def258d4e7ccb6783" } as any);
+        const pl = new SingleProjectLoader(p);
+        const gi = fakeGoalInvocation(id, {
+            projectLoader: pl,
+            repoRefResolver: FakeRepoRefResolver,
+        } as any);
+        const errors: string[] = [];
+        const testFix: AutofixRegistration = {
+            name: "test",
+            transform: async (project, ci) => {
+                if (project !== p) {
+                    errors.push("Project not the same");
+                }
+                if (!ci.push || ci.push.push !== gi.sdmGoal.push) {
+                    errors.push("push should be set");
+                }
+            },
+        };
+        await executeAutofixes([testFix])(gi);
+        assert.deepEqual(errors, []);
+    }).timeout(10000);
+
     describe("filterImmediateAutofixes", () => {
 
         it("should correctly filter applied autofix", () => {