Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not all semver specifications are supported #986

Closed
Janther opened this issue May 26, 2024 · 4 comments · Fixed by #1000
Closed

Not all semver specifications are supported #986

Janther opened this issue May 26, 2024 · 4 comments · Fixed by #1000
Assignees
Labels
bug Something isn't working

Comments

@Janther
Copy link

Janther commented May 26, 2024

Currently this triggers an error when parsed.

pragma solidity >=0.7.0 <=0.8.5;

Here is a detailed example of a grammar that supports all current semver compatible notations.

On another hand, solidity's official lexer just accepts a blob between the tokens pragma and ;

Hopefully this helps.

@Xanewok Xanewok added the bug Something isn't working label May 27, 2024
@OmarTawfik OmarTawfik self-assigned this May 27, 2024
@OmarTawfik
Copy link
Collaborator

@Janther this input is parsing successfully without errors (see below). Can you please clarify? which error are you seeing?

  - (SourceUnit) ► (members꞉ SourceUnitMembers): # "pragma solidity >=0.7.0 <=0.8.5;\n" (0..33)
      - (item꞉ SourceUnitMember) ► (variant꞉ PragmaDirective): # "pragma solidity >=0.7.0 <=0.8.5;\n" (0..33)
          - (pragma_keyword꞉ PragmaKeyword): "pragma" # (0..6)
          - (pragma꞉ Pragma) ► (variant꞉ VersionPragma): # " solidity >=0.7.0 <=0.8.5" (6..31)
              - (leading_trivia꞉ Whitespace): " " # (6..7)
              - (solidity_keyword꞉ SolidityKeyword): "solidity" # (7..15)
              - (sets꞉ VersionExpressionSets): # " >=0.7.0 <=0.8.5" (15..31)
                  - (item꞉ VersionExpressionSet): # " >=0.7.0 <=0.8.5" (15..31)
                      - (item꞉ VersionExpression) ► (variant꞉ VersionComparator): # " >=0.7.0" (15..23)
                          - (leading_trivia꞉ Whitespace): " " # (15..16)
                          - (operator꞉ GreaterThanEqual): ">=" # (16..18)
                          - (operand꞉ VersionExpression) ► (variant꞉ VersionSpecifiers): # "0.7.0" (18..23)
                              - (item꞉ VersionSpecifier): "0" # (18..19)
                              - (separator꞉ Period): "." # (19..20)
                              - (item꞉ VersionSpecifier): "7" # (20..21)
                              - (separator꞉ Period): "." # (21..22)
                              - (item꞉ VersionSpecifier): "0" # (22..23)
                      - (item꞉ VersionExpression) ► (variant꞉ VersionComparator): # " <=0.8.5" (23..31)
                          - (leading_trivia꞉ Whitespace): " " # (23..24)
                          - (operator꞉ LessThanEqual): "<=" # (24..26)
                          - (operand꞉ VersionExpression) ► (variant꞉ VersionSpecifiers): # "0.8.5" (26..31)
                              - (item꞉ VersionSpecifier): "0" # (26..27)
                              - (separator꞉ Period): "." # (27..28)
                              - (item꞉ VersionSpecifier): "8" # (28..29)
                              - (separator꞉ Period): "." # (29..30)
                              - (item꞉ VersionSpecifier): "5" # (30..31)
          - (semicolon꞉ Semicolon): ";" # (31..32)
          - (trailing_trivia꞉ EndOfLine): "\n" # (32..33)

@Janther
Copy link
Author

Janther commented May 27, 2024

when going through the tree, this is the error I get when reaching the level of VersionComparator

const source = `
pragma solidity >=0.7.0 <=0.8.5;
`;

const language = new Language('0.8.25');
new SourceUnit(language.parse(RuleKind.SourceUnit, source).tree())
  .members
    .items[0]
      .variant
        .pragma
          .variant
            .sets
              .items[0]
                .items[0]
                  .variant
                    .operator; // Missing child node at index '1'.

@OmarTawfik
Copy link
Collaborator

Thanks for the repro. Looking into it!

@Janther
Copy link
Author

Janther commented May 30, 2024

Not sure if it's related but I have found a similar error when parsing code with a ComparisonExpression.

There might be an issue when parsing comparison symbols (eg: <=).

Since the same would happen when reading the VersionComparator.

const source = `
contract Comparison {
  function compare(uint a, uint b) public pure returns(bool) {
      return a <= b;
  }
}

`;

const language = new Language('0.8.25');
const parsed = new SourceUnit(language.parse(RuleKind.SourceUnit, source).tree())
  .members
    .items[0]
      .variant
        .members
          .items[0]
            .variant
              .body
                .variant
                  .statements
                    .items[0]
                      .variant
                        .expression
                          .variant
                            .leftOperand; // Missing child node at index '2'.

OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this issue Jun 10, 2024
Uses labels to select AST nodes, which is more accurate, requires less code (instead of checking individual kinds), and also fixes a bug where additional `TerminalKind`s of `PrecedenceOperator` are not taken into account.

Closes NomicFoundation#872
Closes NomicFoundation#986
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this issue Jun 10, 2024
Uses labels to select AST nodes, which is more accurate, requires less code (instead of checking individual kinds), and also fixes a bug where additional `TerminalKind`s of `PrecedenceOperator` are not taken into account.

Closes NomicFoundation#872
Closes NomicFoundation#986
OmarTawfik added a commit to OmarTawfik-forks/slang that referenced this issue Jun 10, 2024
Uses labels to select AST nodes, which is more accurate, requires less code (instead of checking individual kinds), and also fixes a bug where additional `TerminalKind`s of `PrecedenceOperator` are not taken into account.

Closes NomicFoundation#872
Closes NomicFoundation#986
github-merge-queue bot pushed a commit that referenced this issue Jun 11, 2024
Uses labels to select AST nodes, which is more accurate, requires less
code (instead of checking individual kinds), and also fixes a bug where
additional `TerminalKind`s of `PrecedenceOperator` are not taken into
account.

Closes #872
Closes #986
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants