Skip to content

Conversation

pchampin
Copy link
Collaborator

@pchampin pchampin commented Aug 5, 2025

as pointed out by @GordianDziwis, the name IriValid could be read as "valid IRI", implying that the trait Iri could yield invalid IRIs...

see #6 (comment)

as pointed out by @GordianDziwis, the name IriValid could be read as "valid IRI",
implying that the trait Iri could yield invalid IRIs...

see #6 (comment)
@pchampin
Copy link
Collaborator Author

pchampin commented Aug 5, 2025

@GordianDziwis I can not request a review from you, but as this responds to one of your comments, I would appreciate your review on this

@pchampin pchampin changed the title rename traits to avoid ambiguity [statement_validation] rename traits to avoid ambiguity Aug 5, 2025
@GordianDziwis
Copy link

I still think, that factoring out the validation code is a case of premature optimization. For example, in _iri.rs there is the comment:

/// Wrapper around a [`Cow<str>`] guaranteeing that the underlying text satisfies [RFC3987].

The thing is, right now the wrapper does not guarantee that Iri satisfies [RFC3987]!

@pchampin
Copy link
Collaborator Author

pchampin commented Aug 6, 2025

The thing is, right now the wrapper does not guarantee that Iri satisfies [RFC3987]!

The word "guaranteeing" is meant to describe the contract of the type. It is true that the type does not provide any method to enforce the contract, but that does not make the statement wrong, IMO. But I'm open to a more explicit wording, if proposed.

@GordianDziwis
Copy link

The word "guaranteeing" is meant to describe the contract of the type. It is true that the type does not provide any method to enforce the contract, but that does not make the statement wrong, IMO. But I'm open to a more explicit wording, if proposed.

“assuming” is a better fit. The new method from the validation trait then does the "guaranteeing"

@pchampin
Copy link
Collaborator Author

pchampin commented Aug 8, 2025

The word "guaranteeing" is meant to describe the contract of the type. It is true that the type does not provide any method to enforce the contract, but that does not make the statement wrong, IMO. But I'm open to a more explicit wording, if proposed.

“assuming” is a better fit. The new method from the validation trait then does the "guaranteeing"

It really depends where you stand.
As a consumer of an Iri value, you have a guarantee that the underlying text is a valid IRI -- unless, of course, somebody messed up upstream, but separation of concern makes it not your problem.
As a producer, you need to be careful how you construct the IRI: if you use new_unchecked, it is your responsibility to enforce the contract (and the documentation of this constructor does use the verb "assume" as you suggest). If you use new, the contract will be automatically enforced.

That being said, I agree that the current documentation of Iri and LangTag can be confusing, as the word "guarantee" only make sense from the point of view of the consumer, not the producer. I'll proposed an improved text in a separate PR (because I don't want to mix work items in the same PR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants