From db1525ff127d440d1db03667c39bbde69877af6f Mon Sep 17 00:00:00 2001 From: Remy Suen Date: Sat, 19 Aug 2017 22:54:03 +0900 Subject: [PATCH] Fix #132 Warn about invalid build stage names in FROM Signed-off-by: Remy Suen --- CHANGELOG.md | 1 + src/dockerValidator.ts | 51 +++++++++++++++++++++++++----------- test/dockerValidator.test.ts | 46 +++++++++++++++++++++++++------- 3 files changed, 74 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99bc92d..ff2791b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. ### Added - textDocument/publishDiagnostics - warn if ENV or LABEL is missing closing quote ([#143](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/143)) + - warn if FROM's build stage name is invalid ([#132](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/132)) ### Fixed - correct handling of escaped quotes in ENV variables ([#144](https://github.com/rcjsuen/dockerfile-language-server-nodejs/issues/144)) diff --git a/src/dockerValidator.ts b/src/dockerValidator.ts index edd257a..105a9dc 100644 --- a/src/dockerValidator.ts +++ b/src/dockerValidator.ts @@ -26,7 +26,8 @@ export enum ValidationCode { ARGUMENT_REQUIRES_TWO, ARGUMENT_REQUIRES_ONE_OR_THREE, ARGUMENT_UNNECESSARY, - DUPLICATE_NAME, + DUPLICATE_BUILD_STAGE_NAME, + INVALID_BUILD_STAGE_NAME, FLAG_AT_LEAST_ONE, FLAG_DUPLICATE, FLAG_INVALID_DURATION, @@ -140,10 +141,12 @@ export class Validator { for (let i = 0; i < expectedArgCount.length; i++) { if (expectedArgCount[i] === args.length) { for (let j = 0; j < args.length; j++) { - let createInvalidDiagnostic = validate(j, args[j].getValue()); - if (createInvalidDiagnostic) { - let range = args[j].getRange(); - problems.push(createInvalidDiagnostic(range.start, range.end, args[i].getValue())); + let range = args[j].getRange(); + let createInvalidDiagnostic = validate(j, args[j].getValue(), range); + if (createInvalidDiagnostic instanceof Function) { + problems.push(createInvalidDiagnostic(range.start, range.end, args[j].getValue())); + } else if (createInvalidDiagnostic !== null) { + problems.push(createInvalidDiagnostic); } } return; @@ -219,7 +222,7 @@ export class Validator { // duplicates found if (names[name].length > 1) { for (let range of names[name]) { - problems.push(Validator.createDuplicateName(range, name)); + problems.push(Validator.createDuplicateBuildStageName(range, name)); } } } @@ -327,11 +330,20 @@ export class Validator { } break; case "FROM": - this.checkArguments(instruction, problems, [ 1, 3 ], function(index: number, argument: string) { - if (index === 1) { - return argument.toUpperCase() === "AS" ? null : Validator.createInvalidAs; + this.checkArguments(instruction, problems, [ 1, 3 ], function(index: number, argument: string, range: Range) { + switch (index) { + case 1: + return argument.toUpperCase() === "AS" ? null : Validator.createInvalidAs; + case 2: + argument = argument.toLowerCase(); + let regexp = new RegExp(/^[a-z]([a-z0-9_\-.]*)*$/); + if (regexp.test(argument)) { + return null; + } + return Validator.createInvalidBuildStageName(range, argument);; + default: + return null; } - return null; }, Validator.createRequiresOneOrThreeArguments); break; case "HEALTHCHECK": @@ -804,7 +816,8 @@ export class Validator { "syntaxMissingSingleQuote": "failed to process \"${0}\": unexpected end of statement while looking for matching single-quote", "syntaxMissingDoubleQuote": "failed to process \"${0}\": unexpected end of statement while looking for matching double-quote", - "duplicateName": "duplicate name ${0}", + "duplicateBuildStageName": "duplicate name ${0}", + "invalidBuildStageName": "invalid name for build stage: \"${0}\", name can't start with a number or contain symbols", "flagAtLeastOne": "${0} must be at least 1 (not ${1})", "flagDuplicate": "Duplicate flag specified: ${0}", @@ -853,9 +866,13 @@ export class Validator { return Validator.dockerProblems["noSourceImage"]; } - public static getDiagnosticMessage_DuplicateName(name: string) { - return Validator.formatMessage(Validator.dockerProblems["duplicateName"], name); + public static getDiagnosticMessage_DuplicateBuildStageName(name: string) { + return Validator.formatMessage(Validator.dockerProblems["duplicateBuildStageName"], name); } + + public static getDiagnosticMessage_InvalidBuildStageName(name: string) { + return Validator.formatMessage(Validator.dockerProblems["invalidBuildStageName"], name); + } public static getDiagnosticMessage_FlagAtLeastOne(flagName: string, flagValue: string) { return Validator.formatMessage(Validator.dockerProblems["flagAtLeastOne"], flagName, flagValue); @@ -985,8 +1002,12 @@ export class Validator { return Validator.createError(start, end, Validator.getDiagnosticMessage_DirectiveEscapeInvalid(value), ValidationCode.INVALID_ESCAPE_DIRECTIVE); } - private static createDuplicateName(range: Range, name: string): Diagnostic { - return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_DuplicateName(name), ValidationCode.DUPLICATE_NAME); + private static createDuplicateBuildStageName(range: Range, name: string): Diagnostic { + return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_DuplicateBuildStageName(name), ValidationCode.DUPLICATE_BUILD_STAGE_NAME); + } + + private static createInvalidBuildStageName(range: Range, name: string): Diagnostic { + return Validator.createError(range.start, range.end, Validator.getDiagnosticMessage_InvalidBuildStageName(name), ValidationCode.INVALID_BUILD_STAGE_NAME); } static createFlagAtLeastOne(start: Position, end: Position, flagName: string, flagValue: string): Diagnostic { diff --git a/test/dockerValidator.test.ts b/test/dockerValidator.test.ts index 698911b..ecf99e6 100644 --- a/test/dockerValidator.test.ts +++ b/test/dockerValidator.test.ts @@ -418,11 +418,22 @@ function assertSyntaxMissingDoubleQuote(diagnostic: Diagnostic, key: string, sta assert.equal(diagnostic.range.end.character, endCharacter); } -function assertDuplicateName(diagnostic: Diagnostic, instrument: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { - assert.equal(diagnostic.code, ValidationCode.DUPLICATE_NAME); +function assertDuplicateBuildStageName(diagnostic: Diagnostic, name: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { + assert.equal(diagnostic.code, ValidationCode.DUPLICATE_BUILD_STAGE_NAME); assert.equal(diagnostic.severity, DiagnosticSeverity.Error); assert.equal(diagnostic.source, source); - assert.equal(diagnostic.message, Validator.getDiagnosticMessage_DuplicateName(instrument)); + assert.equal(diagnostic.message, Validator.getDiagnosticMessage_DuplicateBuildStageName(name)); + assert.equal(diagnostic.range.start.line, startLine); + assert.equal(diagnostic.range.start.character, startCharacter); + assert.equal(diagnostic.range.end.line, endLine); + assert.equal(diagnostic.range.end.character, endCharacter); +} + +function assertInvalidBuildStageName(diagnostic: Diagnostic, name: string, startLine: number, startCharacter: number, endLine: number, endCharacter: number) { + assert.equal(diagnostic.code, ValidationCode.INVALID_BUILD_STAGE_NAME); + assert.equal(diagnostic.severity, DiagnosticSeverity.Error); + assert.equal(diagnostic.source, source); + assert.equal(diagnostic.message, Validator.getDiagnosticMessage_InvalidBuildStageName(name)); assert.equal(diagnostic.range.start.line, startLine); assert.equal(diagnostic.range.start.character, startCharacter); assert.equal(diagnostic.range.end.line, endLine); @@ -1759,6 +1770,9 @@ describe("Docker Validator Tests", function() { diagnostics = validate("FROM node AS \\ \n setup"); assert.equal(diagnostics.length, 0); + + diagnostics = validate("FROM node AS a_lpi-n.e99"); + assert.equal(diagnostics.length, 0); }); it("invalid as", function() { @@ -1770,18 +1784,32 @@ describe("Docker Validator Tests", function() { it("duplicate name", function() { let diagnostics = validate("FROM node AS setup\nFROM node AS setup"); assert.equal(diagnostics.length, 2); - assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18); - assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18); + assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18); + assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18); diagnostics = validate("FROM node AS setup\nFROM node AS setUP"); assert.equal(diagnostics.length, 2); - assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18); - assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18); + assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18); + assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18); diagnostics = validate("FROM node AS SETUP\nFROM node AS seTUp"); assert.equal(diagnostics.length, 2); - assertDuplicateName(diagnostics[0], "setup", 0, 13, 0, 18); - assertDuplicateName(diagnostics[1], "setup", 1, 13, 1, 18); + assertDuplicateBuildStageName(diagnostics[0], "setup", 0, 13, 0, 18); + assertDuplicateBuildStageName(diagnostics[1], "setup", 1, 13, 1, 18); + }); + + it("invalid name", function() { + let diagnostics = validate("FROM node AS 1s"); + assert.equal(diagnostics.length, 1); + assertInvalidBuildStageName(diagnostics[0], "1s", 0, 13, 0, 15); + + diagnostics = validate("FROM node AS _s"); + assert.equal(diagnostics.length, 1); + assertInvalidBuildStageName(diagnostics[0], "_s", 0, 13, 0, 15); + + diagnostics = validate("FROM node AS a_lpi-n.e,99"); + assert.equal(diagnostics.length, 1); + assertInvalidBuildStageName(diagnostics[0], "a_lpi-n.e,99", 0, 13, 0, 25); }); });