Skip to content

Conversation

@gluax
Copy link
Contributor

@gluax gluax commented Feb 4, 2021

Grammar changes are a bit different than what was suggested in the original feature request #254. However, it should be logically equivalent and I think makes more sense on the rust side.

Closes #254

@gluax gluax requested review from collinc97 and howardwu February 4, 2021 20:22
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #620 (f02845a) into master (770f660) will increase coverage by 0.13%.
The diff coverage is 73.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #620      +/-   ##
==========================================
+ Coverage   75.63%   75.77%   +0.13%     
==========================================
  Files         518      523       +5     
  Lines       16135    16199      +64     
==========================================
+ Hits        12204    12275      +71     
+ Misses       3931     3924       -7     
Impacted Files Coverage Δ
ast/src/imports/package_or_packages.rs 31.81% <31.81%> (ø)
ast/src/imports/package_access.rs 42.85% <40.00%> (+1.47%) ⬆️
ast/src/imports/import.rs 44.44% <66.66%> (ø)
asg/src/program/mod.rs 87.28% <89.47%> (+0.62%) ⬆️
ast/src/imports/packages.rs 90.90% <90.90%> (ø)
grammar/src/imports/import.rs 100.00% <100.00%> (ø)
grammar/src/imports/package_access.rs 100.00% <100.00%> (ø)
grammar/src/imports/package_or_packages.rs 100.00% <100.00%> (ø)
grammar/src/imports/packages.rs 100.00% <100.00%> (ø)
grammar/tests/imports.rs 100.00% <100.00%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 770f660...f02845a. Read the comment docs.

@howardwu howardwu requested a review from acoglio February 4, 2021 22:58

// Declared in imports/import.rs
import = { "import " ~ package ~ LINE_END}
import = { "import " ~ package_type ~ LINE_END}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might consider using package_or_packages instead of package_type, to avoid any potential confusion with Leo types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this, it seems that could help improve readability

acoglio
acoglio previously approved these changes Feb 8, 2021
Copy link
Collaborator

@acoglio acoglio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I just left a very minor comment for consideration in the Pest file.

@gluax gluax linked an issue Feb 9, 2021 that may be closed by this pull request
@collinc97 collinc97 added ast feature A new feature. labels Feb 10, 2021
@collinc97
Copy link
Collaborator

bors r+

bors bot added a commit that referenced this pull request Feb 11, 2021
598: [Feature] 374 circuit self access r=collinc97 a=gluax

Feature resolves #374. At the grammar level the following syntax is no longer allowed on self:
```
self[0];
self();
```

Syntax that is allowed at the grammar level is:
```
console.log("b: {}", self::b);
console.log("hmm: {}",self::hmm());
self.hello();
console.log("access: {}", self.a);
```

Note that these changes are only at the grammar level only.

620: Feature/254 strengthen import rules r=collinc97 a=gluax

Grammar changes are a bit different than what was suggested in the original feature request #254. However, it should be logically equivalent and I think makes more sense on the rust side.

Closes #254 

Co-authored-by: gluaxspeed <[email protected]>
Co-authored-by: Howard Wu <[email protected]>
@collinc97
Copy link
Collaborator

bors r-

@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

Canceled.

@collinc97
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 11, 2021

Build succeeded:

@bors bors bot merged commit b72b5ac into master Feb 11, 2021
@collinc97 collinc97 deleted the feature/254-strengthen-import-rules branch March 31, 2021 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature A new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Strengthen the Pest rules for imports

5 participants