Skip to content

Warn against exported types with hidden constructors but Generic or Newtype instances #3907

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.

Sign up for GitHub

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

Jump to bottom
Merged
hdgarrood merged 2 commits into from
Oct 11, 2020
Merged

Warn against exported types with hidden constructors but Generic or Newtype instances #3907

hdgarrood merged 2 commits into from
Oct 11, 2020

Conversation

@kl0tl
Copy link
Member

@kl0tl kl0tl commented Jul 6, 2020

Close #3707.

kl0tl
kl0tl commented Jul 6, 2020
View reviewed changes
src/Language/PureScript/TypeChecker.hs Outdated
Comment on lines 737 to 738
[ DataNewtype.Newtype
, DataGenericRep.Generic
Copy link
Member Author

@kl0tl kl0tl Jul 6, 2020

Choose a reason for hiding this comment

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

I introduced Language.PureScript.Constants.Data.Generic.Rep and Language.PureScript.Constants.Data.Newtype in order to not duplicate the names of those classes here and during type classes instances deriving but perhaps it would be ok to repeat them?

@kl0tl
Copy link
Member Author

kl0tl commented Jul 12, 2020 •
edited
Loading

This warning is triggered by a few types in the following package set:

https://raw.githubusercontent.com/JordanMartinez/package-sets/fix-warnings-ps-master-compatible/src/packages.dhall sha256:91ea6c8da90935f06b06b60ef6532bfc9f2c1233ec46500d1aab2b27d0dd2b49
  • CSS.Background.BackgroundAttachment
  • CSS.Background.BackgroundClip
  • CSS.Background.BackgroundImage
  • CSS.Background.BackgroundOrigin
  • CSS.Background.BackgroundPosition
  • CSS.Background.BackgroundRepeat
  • CSS.Background.BackgroundSize
  • CSS.Background.Direction
  • CSS.Background.Location
  • CSS.Background.Side
  • CSS.Box.BoxType
  • CSS.Gradient.Extend
  • CSS.Gradient.Radial
  • CSS.Text.Transform.TextTransform
  • CSS.Text.Whitespace.TextWhitespace
  • Data.ByteString.Foldable
  • Data.Float32.Float32
  • Data.TacitString.TacitString
  • Data.UInt.UInt
  • Data.Vec.Vec
  • Hyper.Cookies.MaxAge
  • Identy.ObjectMap.ObjectMap
  • Options.Applicative.Builder.InfoMod
  • Options.Applicative.Builder.PrefsMod
  • Slug.Slug
  • Spork.Html.Core.Html
  • Type.Trout.Links.Link
  • Web.HTML.Window.RequestAnimationFrameId
  • Web.HTML.Window.RequestIdleCallbackId

I can open the necessary pull requests if and when we’ll want to move forward with this.

@kl0tl kl0tl force-pushed the public-hidden-constructors-warning branch from 84592d7 to 342abc7 Compare July 12, 2020 22:34
@kl0tl kl0tl force-pushed the public-hidden-constructors-warning branch from 342abc7 to 6b89a20 Compare September 8, 2020 21:48
This was referenced Sep 9, 2020
Merged
Merged
@hdgarrood hdgarrood merged commit 4d6680e into purescript:master Oct 11, 2020
@kl0tl kl0tl deleted the public-hidden-constructors-warning branch October 11, 2020 19:07
This was referenced Nov 19, 2020
Merged
Merged
@kl0tl kl0tl mentioned this pull request Feb 9, 2021
Merged
@jamesdbrock jamesdbrock mentioned this pull request Jun 22, 2021
Closed
@JordanMartinez JordanMartinez mentioned this pull request Jan 18, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@hdgarrood hdgarrood hdgarrood approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Warn on derived Newtype & Generic instances for exported types with non-exported constructors

2 participants

@kl0tl @hdgarrood