This isn't right, you're changing a replace operation to an append operation.

not really, I just changed how it looks like 😉
But result are the same: addScript() will do the same as array_replace(), if the $url exists, then it "replace", otherwise will "append"

You're relying on a non-documented implementation detail to make this claim. The interface does not document whether the expected behavior is to overwrite an existing entry or raise an error, therefore if someone opted to subclass the Document class and throw an Exception if someone tries to register a URL twice, this assumption is broken.

At the end of the day, you're still changing the logic behind all of this. Right now it is crystal clear you are doing a replace operation. This refactoring looses clarity and proves some of the issues others have highlighted with this PR making an arbitrary assumption that the properties should only have getters.

Indeed if the direct use of public properties $_scripts and $_styleSheets is removed, at least there must be both getters and setters for those properties. Otherwise it won't be no longer possible to manipulate loaded scripts and stylesheets.

The interface does not document whether the expected behavior is to overwrite an existing entry or raise an error

I think, if there no throws tag in a "method description" then there should not be Exception.
if someone throws an Exception then his implementation of the method are incorrect, no?
And duplication should be avoided by Document class, in one or another way, anyway.

Not every throwing method has a documented @throws annotation, and actually a lot of interfaces in general fail to document "allowed" exception types (most of this isn't only related to Joomla, but since most people here don't touch PHP outside of it...). Likewise, not every method has @throws annotations if it doesn't catch exceptions from methods it calls (you then get into a debate whether the method should document only the exceptions it natively throws or whether it should document both those it throws and those that may be thrown by methods it calls, and at that point good luck keeping up with things). Therefore, relying on @throws annotations to indicate whether a method is allowed to throw an exception is not practical.

Either way, the point still stands there is no documented interface behavior, there is only the present de facto standard in the root class (which if this is what people want the method to do, that's fine, document it as the expected interface behavior (as that's exactly what an API specification is), but don't rely on "well this is what the root class does now so this is what will always happen"). Doc blocks are supposed to be more useful than "Adds a linked script to the page", but people are afraid to write useful documentation and instead view doc blocks as something that's just checking the block to pass PHPCS and write useless and doc blocks that are nothing more than a repetition of what the method signature says.

@Fedik just document in the method description the behaviour. I think the chances of people overriding this is fairly low and I'm happy to just make that description explicit rather than being an implementation detail.

@wilsonge I have already updated it in the last commit 😉

@mbabker this is what you meant? :)