Remove usage of form_set_value().

Is it possible function form_set_value() needs to be removed from form.inc in order to pass testing? I've rerolled the patch to find out.

Since the patch is passing all but two tests, let's leave $this->assertText() $text and $message on lines 39 and 48 in class ValidationTest unchanged to see if the patch passes testing. If it passes, then we can look into updating the test.

I also needed to make a few changes to the patch because file.module and SiteInformationForm.php have changed since the patch was rolled 16 days ago. The patch would not apply for me without those changes.

I think I know what went wrong with my last patch. One more try.

That's better, but it's still failing on the class ValidationTest on lines 39 and 48. I think it's something trivial like assertText() doesn't accept the dollar symbol or the hash symbol in its parameters. I looked at a lot of searches for assertText(' and none of the ones I looked at have either of those symbols. I took out $this-> and () in the parameters for a little extra safety, but left the colon since I did see a colon in another test.

Let's try it one more time. Apologies for all the patches. I'm still learning phpunit and our testing suite.

The actual output in test is:
Name value: value changed by $form_state->setValueForElement() in #element_validate

So probably you need to change strings that are used in assert

+++ b/core/modules/system/src/Tests/Form/ValidationTest.php
@@ -36,7 +36,7 @@ function testValidate() {
-    $this->assertText('Name value: value changed by form_set_value() in #element_validate', 'Form element value in $form_state was altered.');
+    $this->assertText('Name value: value changed by setValueForElement in element_validate', 'Form element value in setValueForElement was altered.');

@@ -45,7 +45,7 @@ function testValidate() {
-    $this->assertText('Name value: value changed by form_set_value() in #validate', 'Form element value in $form_state was altered.');
+    $this->assertText('Name value: value changed by setValueForElement in validate', 'Form element value in setValueForElement was altered.');

+++ b/core/modules/system/tests/modules/form_test/src/Callbacks.php
@@ -23,7 +23,7 @@ public function validateName(&$element, FormStateInterface $form_state) {
-      form_set_value($element, 'value changed by form_set_value() in #element_validate', $form_state);
+      $form_state->setValueForElement($element, 'value changed by $form_state->setValueForElement() in #element_validate');

+++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php
@@ -64,7 +64,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
-      form_set_value($form['name'], 'value changed by form_set_value() in #validate', $form_state);
+      $form_state->setValueForElement($form['name'], 'value changed by $form_state->setValueForElement() in #validate');

This tests are failed because of one of this changes

Removed usage of deprecated function form_set_value()
https://github.com/podarok/csua_d8/pull/9

Fixed bugs that failed tests.

#20 looks good

proper tags, +1 rtbc

PS: @vadim.hirbu it's a bit hard to review this kind of patch so checked manually after apply, the better way to do that is to made a "squash" commit or just simply git diff 8.0.x> file.patch

yeah, a git diff is the more common way for patches to be made. The way we express what changes we have done along the way is with interdiffs.
https://drupal.org/documentation/git/interdiff

I pulled on 8.0.x, made a branch, and applied the patch, then I did a git diff to make the simple patch file. (23a.patch)

While looking at this to see the result of evaluating this for if it is allowed in the beta, I noticed that some of the changed lines are now over 80 chars.

"Lines containing comments (including docblocks) must wrap as close to 80 characters as possible without going over" https://www.drupal.org/node/1354#drupal

I fixed a couple lines that were being already changed by this patch (not just fixing everywhere. we want to keep the changes just to lines this needed to fix this issue), and make an interdiff for that.

+++ b/core/modules/system/src/Tests/Form/ValidationTest.php
@@ -25,7 +25,7 @@ class ValidationTest extends WebTestBase {
-   * Tests form alterations by #element_validate, #validate, and form_set_value().
+   * Tests form alterations by #element_validate, #validate, and setValueForElement().

this wasn't explicitly testing form_set_value anyway. (I looked at the test module and the callbacks with xjm) So simplifying, and making it more accurate nicely also makes it less than 80 chars.

---

Also, reviewers, note that:
git diff --color-words
is helpful when looking at a patch like this.

Thanks for rerolling with git diff! I confirmed that the git diff patch includes the same changes as the earlier patch. The changes in the interdiff to clean up the documentation also wrapping look correct.

+++ b/core/modules/system/src/Tests/Form/ValidationTest.php
@@ -25,7 +25,7 @@ class ValidationTest extends WebTestBase {
-   * Tests form alterations by #element_validate, #validate, and setValueForElement().
+   * Tests #element_validate and #validate.

@YesCT and I looked over the code for this together and I agree that this is a less misleading docblock for this method. @YesCT also confirmed that this removes all usages of the deprecated function in core.

I also evaluated https://www.drupal.org/contribute/core/beta-changes with this issue.

Since it is a normal task,
this issue is allowed in 8.0.x under prioritized type:
code already marked for removal by 8.0.0

and it was and is:

/**
 * Changes submitted form values during form validation.
 *
 * @deprecated in Drupal 8.x-dev, will be removed before Drupal 8.0.
 *   Use $form_state->setValueForElement().
 *
 * @see \Drupal\Core\Form\FormStateInterface::setValueForElement().
 */
function form_set_value($element, $value, FormStateInterface $form_state) {
  $form_state->setValueForElement($element, $value);
}

So, then we have to see if the impact is greater than the disruption.

From that beta changes handbook page:

"A change is disruptive if it:

Introduces a BC break that will affect many contributed modules, or require some contributed modules to make non-trivial changes
Will require internal refactoring or rewriting of core subsystems, as these changes tend to introduce technical debt and regressions
Will require widespread documentation or code style updates which are likely to conflict with other patches in the queue
Will require changes across many subsystems and patches in order to make core consistent."

It does not introduce a BC break.
It will not require refactoring or rewriting of core systems
Will not cause too many other patches in the queue to be rerolled. (patch is not giant, and changes are isolated)
This gets all of the changes.

So, I think this should be allowed in the beta.

(copying this to the summary)

--

also checked if this got all of the usages.
This ag command was useful:

ag 'form_set_value\(' | wc -l

This issue is a prioritized change (deprecated function) as per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase? and it's benefits outweigh any disruption. Committed 789de47 and pushed to 8.0.x. Thanks!