rcjsuen / dockerfile-language-server Goto Github PK

#=# key=value
FROM node

The parser has code to skip over invalid parser directive definitions. However, if the first comment starts with a = character then it doesn't end up consuming the rest of the line correctly.

The MAINTAINER instruction is currently deprecated so the validator should warn the client about this. However, some clients may wish to ignore the fact that it is deprecated. Alternatively, they may wish to have errors created if such an instruction exists in the Dockerfile. We should introduce a setting in the validator to allow this to be toggled if desired.

Travis Build #84 is broken.

$ npm run build;
> [email protected] build /home/travis/build/rcjsuen/dockerfile-language-server-nodejs
> tsc -p .
parser/dockerfile.ts(10,21): error TS2307: Cannot find module './instructions/arg'.
parser/dockerfileParser.ts(12,21): error TS2307: Cannot find module './instructions/arg'.

When a file does not have a FROM instruction as its first instruction or when the Dockerfile doesn't have any instructions, we should use one common error to notify the user that a source image must be specified.

The validator is no longer being when a document has been opened or modified.

Travis Builds #66 and #67 are broken because of a compiler error.

> [email protected] build /home/travis/build/rcjsuen/dockerfile-language-server-nodejs
> tsc -p .
src/dockerValidator.ts(12,35): error TS2307: Cannot find module './dockerValidatorSettings'.

The MAINTAINER instruction is deprecated and causes every single test that involves that instruction to have at least one diagnostic pop up. We need to refactor the tests to consider this case.

At the moment, every response handler parses a Dockerfile with its own unique code. While a Dockerfile's syntax is not exactly very complicated, a lot of code that is very similar is replicated across the entire codebase. We should rewrite a parser from scratch that will create an AST of sorts that knows where a comment, directive, and instruction line starts and ends (including any escaped newlines in the case of instructions).

To validate that this parser is working as intended, it will first be used to replace dockerSymbols.ts before further refactorings across the entire codebase.

FROM node AS bootstrap
COPY bootstrap /git/build/

If you have a Dockerfile with COPY instructions that don't specify a --from= flag, an error will be thrown.

[Error - 7:52:20 PM] Request textDocument/documentHighlight failed.
  Message: Request textDocument/documentHighlight failed with message: Cannot read property 'start' of null
  Code: -32603 

The Dockerfile below does not get warned about the incorrectly named instruction \FROM.

\FROM node

Currently, the message is:

Invalid stop signal

We should use the same message that Docker gives.

Invalid signal: a

The first build has failed. It seems that it's because the .travis.yml file is referencing an environment variable that doesn't exist.

FROM node A$ setup

Given this Dockerfile, the second argument A$ should be flagged as an error.

Given a Dockerfile with build stages, a user that selects the name in a COPY instruction should be able to jump to the build stage definition in the corresponding FROM instruction.

Travis Build #42

$ npm run build;
> [email protected] build /home/travis/build/rcjsuen/dockerfile-language-server-nodejs
> tsc -p .
src/dockerValidator.ts(292,8): error TS7027: Unreachable code detected.

If the second argument of a FROM is not an AS, a diagnostic will flag it as an error. We should create a code action for converting the argument to an AS.

#escape=      

Given a Dockerfile with an escape parser directive that has a string of whitespace characters after it, the whitespace characters should be flagged as a validation error.

The last instruction or comment of a file is not being handled correctly in some cases due to the parser not terminating the parsing when encountering an EOF.

# escape= `

FROM

With the introduction of build stages, we should add rename support so that references to a build stage name in a FROM and a COPY will get be renamed by the client with a single action.

FR\om node

Given a Dockerfile like this, the symbol information should flag FR\OM as the symbol's name but it currently considers the name to be FROM.

FROM node AS bootstrap
FROM node
COPY --from=bootstrap

Given this Dockerfile, features like textDocument/documentHighlight, textDocument/rename, and textDocument/definition all don't work.

According to moby/moby #32074, malformed JSON arrays don't get flagged as errors as they get interpreted as shell syntax instead. As-such, we should remove the JSON parsing code from the validator because basically "anything goes" for instructions like RUN and VOLUME.

Due to a typo, invalid stop signals currently don't have a message associated with the failure diagnostic.

FROM node
#expose 8080

A Dockerfile like the above will cause the validator to flag the expose as being an instruction that needs to be written in uppercase. Since it's in a comment, the parser should simply be ignoring that line.

The HEALTHCHECK NONE instruction works as-is and should not be modified by the user in anyway. As such, there is no reason for it to be a snippet.

We should make the necessary changes to package.json to prepare the project for publishing to npm.

If an escaped character is nested within an instruction directly without any newline characters, no hover information should be returned.

FR\om busybox

FR\oom busybox

Originally, FROM instructions only took a single argument of the following forms:

1. FROM <image>
2. FROM <image>[:<tag>]
3. FROM <image>[@<digest>]

However, a new AS keyword has been introduced and a FROM instruction can now be defined by something like FROM node AS bootstrap.

As the validator currently validates FROM as an instruction that may only have one argument, we need to update it to to accept either one or three arguments (with the second being an AS).

FROM node
RUNCMD docker\

This Dockerfile with three lines will cause the validator to loop indefinitely.

FROM node

If you try to format this document with a valid range, the server will return one TextEdit that replaces content from Position(0, 0) to Position(0, 0) to the empty string "". This is extraneous and we should simply return an empty TextEdit[] in this case.

A new general-purpose parser for creating an AST-like structure for a Dockerfile was implemented in #24. We should migrate our existing parsing code to use this generic parser instead.

Travis Build #33

> [email protected] build /home/travis/build/rcjsuen/dockerfile-language-server-nodejs
> tsc -p .
test/dockerValidator.tests.ts(678,1): error TS1128: Declaration or statement expected.
test/dockerValidator.tests.ts(678,2): error TS1128: Declaration or statement expected.

The range being specified is currently off because it uses the same value as its starting position and its ending position.

For a HEALTHCHECK instruction, two completion items are returned to the client. One for the RUN case and the other for the NONE case. If snippets are not available on the client, both completion items will just insert HEALTHCHECK into the editor. It should insert HEALTHCHECK NONE into the editor for the NONE case.

While applying edits from the client from a textDocument/didChange request, the substring analysis code corrupts the internal representation of the document. This causes subsequent requests to the server to return incorrect information.

If the first instruction is not a FROM, the validator will flag it as an error. However, if it seems that FROM instructions can actually be preceded by an ARG.

We need to fix the validator to consider this case so that ARG instruments that appear before a FROM do not get flagged as an error.

The parser is not taking into account what happens if it runs to the beginning of the file in a nested case.

The bug can be seen if you try to hover over the EXPOSE keyword.

RUN EXPOSE 8080

The handler for textDocument/rangeFormatting is currently incomplete. It only handles the case where the selection is on a single line. It should also work for a selection that spans multiple lines.

HEALTHCHECK\
NONE

Given this incorrect Dockerfile, two symbols are returned when there should only be one. The escaped newline should merge the instruction into one HEALTHCHECKNONE instruction.

The symbols parsing code currently incorrectly trims out the leading whitespace of the document.

# escape=`

This file with a blank newline at the beginning will cause the symbol's range to be incorrect.

We should leverage the Istanbul nyc project and publish its code coverage results to Coveralls.

Travis Build #56

$ npm run build;
> [email protected] build /home/travis/build/rcjsuen/dockerfile-language-server-nodejs
> tsc -p .
parser/instruction.ts(8,26): error TS2307: Cannot find module './argument'.

#

If a completion request is attempted on the above Dockerfile, no suggestions will be provided. It should suggest the escape parser directive as an option.

Travis CI Build #32

  Docker Validator Tests
    missing argument
      ✓ ADD
      ✓ ARG
      ✓ CMD
      ✓ COPY
      ✓ ENTRYPOINT
      ✓ ENV
      ✓ EXPOSE
      ✓ FROM
      ✓ HEALTHCHECK
      ✓ LABEL
      1) MAINTAINER
      ✓ ONBUILD
      ✓ RUN
      ✓ SHELL
      ✓ STOPSIGNAL
      ✓ USER
      ✓ WORKDIR

  1) Docker Validator Tests missing argument MAINTAINER:
      AssertionError [ERR_ASSERTION]: 0 == 3
      + expected - actual
      -0
      +3
      
      at assertInstructionMissingArgument (out/test/dockerValidator.tests.js:1:46748)
      at testMissingArgument (out/test/dockerValidator.tests.js:1:59678)
      at Context.<anonymous> (out/test/dockerValidator.tests.js:1:62522)

The MAINTAINER-specific parsing code only flags the keyword itself as being deprecated. It should also check to see if it's actually followed by an argument and to create a MISSING_ARGUMENT diagnostic if it's not.

Given the following Dockerfile, hovering over the FROM instruction will not return anything.

FR\
OM node

At the moment, only three types of diagnostics have codes, LOWERCASE, EXTRA_ARGUMENT, and INVALID_ESCAPE_DIRECTIVE. We should add codes for the remaining diagnostics.

FROM alpine as setup
FROM alpine
COPY --from=

Given this Dockerfile, we should suggest setup as a completion item. Suggesting 1 is questionable as you're not supposed to refer to the current build stage but perhaps it is worth suggesting for the sake of completeness.

For the initial pass, we will need tests that test the actual leg work of each underlying feature. In the future, a separate set of tests should be created to make sure that the server is returning the right responses back to the client.

• code actions
• command execution
• completion
• diagnostics
• document symbols
• formatting
• hover

If a user has build stages defined (as of Docker 17.06 CE), we should provide highlighting support if the build stage declaration itself in the FROM instruction or the name in the --from=name of the COPY instruction is currently selected.

On ranged selection formatting, if \r or \r\n are used for newlines and are actually escaped, the start of the line is not being calculated correctly and the formatter deems that no formatting is required.

FROM node
EXPOSE 8080\
8081

Here, 8081 should get indented if it is selected but the formatter gets confused and doesn't return a TextEdit for this scenario.