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

fix: add duplicate name check in Substance checker #657

Merged
merged 2 commits into from Oct 7, 2021
Merged

Conversation

wodeni
Copy link
Member

@wodeni wodeni commented Oct 5, 2021

Description

Related issue/PR: #654

The Substance checker doesn't check for duplicated names (since the Haskell days!?). This PR adds duplicate name checks for Decls.

Implementation strategy and design decisions

@wodeni wodeni requested a review from samestep October 5, 2021 16:01
@wodeni wodeni self-assigned this Oct 5, 2021
@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #657 (69e6b7e) into main (a50ccae) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #657      +/-   ##
==========================================
+ Coverage   66.54%   66.56%   +0.01%     
==========================================
  Files          44       44              
  Lines        7106     7109       +3     
  Branches     1337     1338       +1     
==========================================
+ Hits         4729     4732       +3     
  Misses       2368     2368              
  Partials        9        9              
Impacted Files Coverage Δ
packages/core/src/index.ts 77.51% <ø> (-0.18%) ⬇️
packages/core/src/compiler/Substance.ts 90.47% <100.00%> (+0.12%) ⬆️

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 a50ccae...69e6b7e. Read the comment docs.

@wodeni wodeni linked an issue Oct 6, 2021 that may be closed by this pull request
Copy link
Collaborator

@samestep samestep left a comment

Choose a reason for hiding this comment

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

lgtm other than a couple random questions

packages/core/src/index.ts Show resolved Hide resolved
packages/core/src/compiler/Substance.ts Outdated Show resolved Hide resolved
@wodeni wodeni merged commit 1a3df91 into main Oct 7, 2021
@wodeni wodeni deleted the fix-duplicate-ids branch October 7, 2021 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Substance checker not catching name collisions across types
2 participants