Reenable warning 22 across the codebase #14361
Conversation
|
!ci-build-me |
| , Global_slot_span.Stable.V1.t | ||
| , Balance.Stable.V1.t | ||
| , Amount.Stable.V1.t ) | ||
| , Global_slot_since_genesis.Stable.Latest.t |
There was a problem hiding this comment.
Can these types omit the Stable.Latest, so Global_slot_since_genesis.t, for example. That's the same type.
| ( Payload.Stable.V2.t | ||
| , Public_key.Stable.V1.t | ||
| , Signature.Stable.V1.t ) | ||
| ( Payload.Stable.Latest.t |
There was a problem hiding this comment.
I don't think we should do this. A versioned type should be built from other specific versioned types. If those other types add new versions, this type will change.
It is true that the new version linter will detect such changes, but still.
There was a problem hiding this comment.
It is also true that this is a signature, not an implementation. But still ...
There was a problem hiding this comment.
I changed these to get around the ppx_version warning, but have since disabled it, so I can undo all of these changes to refer to the latest versions.
| let versioned_type_misuse_errors_fun = | ||
| if not @@ in_versioned_type_module acc.module_path then | ||
| get_versioned_type_misuses | ||
| else if not acc.in_versioned_ext then not_in_versioned_ext_fun |
There was a problem hiding this comment.
Even if you allow mentions of specific versions, don't you want to keep the lint warning when a versioned type is used without %%verioned?
For example, this type declaration currently generates a warning:
module Foo = struct
module Stable = struct
module V1 = struct
type t = int
end
end
end
There was a problem hiding this comment.
Ok. So then we still want to return not_in_versioned_ext_fun if in_versioned_type_module acc.module_path && not acc.in_versioned_ext?
|
!ci-build-me |
|
!ci-build-me |
|
!approved-for-mainnet |
Disabling warning 22 hides a variety of important warnings, including warnings regarding unused variables. This PR reenables this warning across all libraries, and patches up those libraries so that no warnings are triggered. As part of this, I disabled a lint in ppx_version where we do not allow stable type versions other than
Stable.Latestto be referenced outside of[%%versioned: ...]blocks. I do not believe this check is necessary, and there are many places in the codebase where we are interested in referencing the old type versions.