Skip to content

ast/parser: vairables in port ranges#85

Merged
lorenzo merged 1 commit intohadolint:masterfrom
m-ildefons:876-port-range
Nov 9, 2022
Merged

ast/parser: vairables in port ranges#85
lorenzo merged 1 commit intohadolint:masterfrom
m-ildefons:876-port-range

Conversation

@m-ildefons
Copy link
Copy Markdown
Member

  • Fix parser being unable to parse port ranges when the limits are expressed with variables
  • Rearrange AST such that the limits of port ranges are ports
  • Move tests for EXPOSE instructions to their own file

This change introduces a breaking change to the AST.

The old AST was unable to represent port ranges delimited with variables, since the data type for port ranges mandated that a limit must be expressed as an integer. This creates problems when limits of a range are expressed as variables (e.g. as ARGs). The change to the AST defines the limits of a port range as port. This has the advantage that any way a port can be expressed can also be used to express a port range limit, including variables.

required-for: hadolint/hadolint#876

Since this would be a breaking change in the AST, I'd recommend a major version jump. This will require adjustments to Hadolint, but offers the ability to fix hadolint/hadolint#876

- Fix parser being unable to parse port ranges when the limits are
  expressed with variables
- Rearrange AST such that the limits of port ranges are ports
- Move tests for `EXPOSE` instructions to their own file

This change introduces a breaking change to the AST.

The old AST was unable to represent port ranges delimited with
variables, since the data type for port ranges mandated that a limit
must be expressed as an integer. This creates problems when limits of a
range are expressed as variables (e.g. as ARGs). The change to the AST
defines the limits of a port range as port. This has the advantage that
any way a port can be expressed can also be used to express a port range
limit, including variables.

required-for: hadolint/hadolint#876
@m-ildefons m-ildefons requested a review from lorenzo October 5, 2022 12:36
@lorenzo
Copy link
Copy Markdown
Member

lorenzo commented Nov 9, 2022

Thanks!

@lorenzo lorenzo merged commit e51ca63 into hadolint:master Nov 9, 2022
@mloskot
Copy link
Copy Markdown

mloskot commented Nov 10, 2022

Thanks!

m-ildefons added a commit to m-ildefons/hadolint that referenced this pull request Nov 10, 2022
Fix up usage of the AST in DL3011 corresponding to the changes to the
AST with respect to port ranges in `EXPOSE` instructions.

requires: hadolint/language-docker#85
fixes: hadolint#876
lorenzo pushed a commit to hadolint/hadolint that referenced this pull request Nov 16, 2022
Fix up usage of the AST in DL3011 corresponding to the changes to the
AST with respect to port ranges in `EXPOSE` instructions.

requires: hadolint/language-docker#85
fixes: #876
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unexpected '$' expecting '`', a new line followed by the next instruction, or the variable name

3 participants