Tidy up of LanguageInterface - removal of setters and other unnecessary methods

Updated issue summary, it said setValuesFromValues while it is setValuesFromLanguage.

I am wondering that now that so much was done to unify the two interfaces / implementations, why do we need the intermediary Language object when we save a new ConfigurableLanguage?

Also re the locked property, a ConfigurableLanguage may not be in fact configurable on the UI IF it is locked. Drupal core ships with 2 non-configurable languages as config entities (AKA ConfigurableLanguage :D). Naming things is hard right? :)

Yeah this looks like an amazing intermediary step then :) I am a huge proponent of not doing everything in one issue :)

Wait, the method_id stuff. core/modules/language/tests/language_test/language_test.module still uses it to test if the right method was used and that is not affected by this patch. Also surprising it passes with removal of that. How can that be?

I wish we had #2057905: [policy, no patch] Discuss the standards for phpunit based tests and #2253915: [policy, no patch] Fix coding standards for PHPUnit tests in a better state.

maybe a separate issue to handle

+++ b/core/lib/Drupal/Core/Language/LanguageDefault.php
@@ -53,7 +53,7 @@ public function get() {
* The default language.
*/
public function set(LanguageInterface $language) {
- $language->default = TRUE;
+ $language->setDefault(TRUE);
$this->language = $language;
}

default will be a protected in #2226533: Changes to the Language class due to the LanguageInterface (followup) and this issue takes out the setDefault().

Maybe we should not have the default property? since... a recent discussion has said that if a language is default is not stored on the language object. #2336177: ConfigurableLanguage cannot be created/removed directly, integrate language_save() and language_delete()

similar #2337835: set language->default in ConfigLanguageOverrideWebTest

similar #2337843: LanguageEditForm take out set name and direction, use create() parameters

slightly related to an irc conversation we had about language tests: #2337943: make a language test base helper that makes a random langcode (not in scope of this issue though)

I am not expressing an opinion one way or the other, but just to say I appreciate an informed discussion,

I have a use case for keeping some get/setters in \Drupal\language\Entity\ConfigurableLanguage

namely setDirection() and setName()

please see https://www.drupal.org/node/2337843#comment-9146131 which discusses how to transform \Drupal\language\Form\LanguageEditForm::submitForm()

  public function submitForm(array &$form, FormStateInterface $form_state) {
    // Prepare a language object for saving.
    $languages = language_list();
    $langcode = $form_state->getValue('langcode');
    $language = $languages[$langcode];
    $language->name = $form_state->getValue('name');
    $language->direction = $form_state->getValue('direction');
    language_save($language);

    $form_state->setRedirect('language.admin_overview');
  }

@martin107: #2336177: ConfigurableLanguage cannot be created/removed directly, integrate language_save() and language_delete() makes much more sweeping changes, no need for set methods. See there.

That just landed, so this needs a reroll.

So language negotiation would only happen if language module is turned on, right? Otherwise all language types are resolved to the default language. In which case can a Lanaguage object (as opposed to ConfigurableLanguage) be result of language negotiation and therefore need the method_id?

Oh wait, I see it was on both classes BEFORE the patch. I guess that was for API compatibility because the getter/setter was on the interface. Since you want to remove this anyway, there is little point debating it on this issue, and you just restored a non-modified situation regarding those properties. So good then. Let's get this in too :)

Committed/pushed to 8.0.x, thanks!

Yay, thanks!

Why exactly was stuff like setWeight() removed from ConfigurableLanguage? I agree that it doesn't make sense on Language, but on ConfigurableLanguage? In the unit tests, the calls to setWeight() are being replaced with e.g. new ConfigurableLanguage(['weight' => 0]) which is far worse DX than $language->setWeight(). I didn't check how the UI sets the language weights.

I would like to open an issue to revert that change but am wondering if there are some specific arguments that I am missing here.

Well, in my book setWeight() > set('weight'), but sure I'll open a follow-up, it's not a big deal either way. Just wanted to know if there was anything I missed. Thanks for the clarification!

#2341341: Change public 'name' property access on languages to getName() and add back setName() is adding back a setName()