Skip to content

Commit

Permalink
Fix #57 Correct isBuildVariable() for variables in FROM
Browse files Browse the repository at this point in the history
Signed-off-by: Remy Suen <remy.suen@gmail.com>
  • Loading branch information
rcjsuen committed May 21, 2019
1 parent 6261a8c commit 71cb909
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file.
## [Unreleased]
### Fixed
- variables in `FROM` should return `true` for `isDefined()` if it is defined in the `ARG` instructions above it ([#56](https://github.com/rcjsuen/dockerfile-ast/issues/56))
- variables in `FROM` should return `true` for `isBuildVariable()` if it is defined in the `ARG` instructions above it ([#57](https://github.com/rcjsuen/dockerfile-ast/issues/57))

## [0.0.14] - 2019-04-28
### Added
Expand Down
12 changes: 12 additions & 0 deletions src/instruction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { Dockerfile } from './dockerfile';
import { Line } from './line';
import { Argument } from './argument';
import { Variable } from './variable';
import { Keyword } from './main';
import { Arg } from './instructions/arg';

export class Instruction extends Line {

Expand Down Expand Up @@ -569,6 +571,16 @@ export class Instruction extends Line {
}

private isBuildVariable(variable: string, line: number): boolean | undefined {
if (this.getKeyword() === Keyword.FROM) {
for (const initialArg of this.dockerfile.getInitialARGs()) {
const arg = initialArg as Arg;
const property = arg.getProperty();
if (property && variable === property.getName()) {
return true;
}
}
return undefined;
}
let image = this.dockerfile.getContainingImage(Position.create(line, 0));
let envs = image.getENVs();
for (let i = envs.length - 1; i >= 0; i--) {
Expand Down
33 changes: 33 additions & 0 deletions test/variable.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,38 @@ describe("Variable", () => {
assert.equal(variable.toString(), "$var");
});

it("undefined ARG variable before FROM", () => {
let dockerfile = DockerfileParser.parse("ARG\nFROM $image");
let variable = dockerfile.getInstructions()[1].getVariables()[0];
assert.equal(variable.getName(), "image");
assertRange(variable.getNameRange(), 1, 6, 1, 11);
assertRange(variable.getRange(), 1, 5, 1, 11);
assert.equal(variable.getModifier(), null);
assert.equal(variable.getModifierRange(), null);
assert.equal(variable.getSubstitutionParameter(), null);
assert.equal(variable.getSubstitutionRange(), null);
assert.equal(variable.isDefined(), false);
assert.equal(variable.isBuildVariable(), false);
assert.equal(variable.isEnvironmentVariable(), false);
assert.equal(variable.toString(), "$image");
});

it("declared ARG variable before FROM", () => {
let dockerfile = DockerfileParser.parse("ARG image\nFROM $image");
let variable = dockerfile.getInstructions()[1].getVariables()[0];
assert.equal(variable.getName(), "image");
assertRange(variable.getNameRange(), 1, 6, 1, 11);
assertRange(variable.getRange(), 1, 5, 1, 11);
assert.equal(variable.getModifier(), null);
assert.equal(variable.getModifierRange(), null);
assert.equal(variable.getSubstitutionParameter(), null);
assert.equal(variable.getSubstitutionRange(), null);
assert.equal(variable.isDefined(), true);
assert.equal(variable.isBuildVariable(), true);
assert.equal(variable.isEnvironmentVariable(), false);
assert.equal(variable.toString(), "$image");
});

it("defined ARG variable to use in FROM", () => {
let dockerfile = DockerfileParser.parse("ARG image=node\nFROM $image");
let variable = dockerfile.getInstructions()[1].getVariables()[0];
Expand All @@ -650,6 +682,7 @@ describe("Variable", () => {
assert.equal(variable.getSubstitutionParameter(), null);
assert.equal(variable.getSubstitutionRange(), null);
assert.equal(variable.isDefined(), true);
assert.equal(variable.isBuildVariable(), true);
assert.equal(variable.isEnvironmentVariable(), false);
assert.equal(variable.toString(), "$image");
});
Expand Down

0 comments on commit 71cb909

Please sign in to comment.