Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); [release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue by github-actions[bot] · Pull Request #91293 · dotnet/runtime · GitHub
Skip to content

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue - #91293

Merged
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0
Aug 31, 2023
Merged

[release/8.0] Fix nullable annotation for Validator.TryValidateValue and ValidateValue#91293
carlossanlop merged 4 commits into
release/8.0from
backport/pr-91286-to-release/8.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Aug 29, 2023

Copy link
Copy Markdown
Contributor

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, the source generator can potentially stop emitting the ! into the generated code when targeting net8+. (That change is being considered but is not included in this PR).

Testing

Compilation and unit tests succeed. A search across other dotnet org repos found no call sites to update.

Risk

Low. Only affects nullable annotation behavior at design-time, and this relaxes an erroneous annotation in a non-breaking way.

@ghostghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners new-api-needs-documentation labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@carlossanlopcarlossanlop added area-System.ComponentModel.DataAnnotations and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Aug 29, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-system-componentmodel-dataannotations
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #91286 to release/8.0

/cc @jeffhandley

Customer Impact

System.ComponentModel.DataAnnotations.Validator has methods for validating individual values against a list of validation attributes; they are ValidateValue and TryValidateValue. Those two methods are annotated as not allowing null values to be validated, and the XML doc comments indicate the value cannot be null as well. The annotation and comment were incorrect though, with the XML doc comment likely copied and pasted from the Object validation members long ago.

Customers haven't historically called these methods directly, with it being more of a framework-level concern to orchestrate validation. With the Options Validation source generator introduced in .NET 8 though, we are now generating code into customers' projects that calls these methods directly so that Reflection can be bypassed at runtime. With the incorrect nullable annotation, we have to generate code that includes a ! on the argument value. Taking this fix in .NET 8, we can stop generating the superfluous ! as customers adopt the source generator.

Testing

Pending: #91286 (comment)

Risk

Author:github-actions[bot]
Assignees:-
Labels:

area-System.ComponentModel.DataAnnotations, new-api-needs-documentation

Milestone:-

@jeffhandleyjeffhandley added the Servicing-approved Approved for servicing release label Aug 31, 2023
@jeffhandley

Copy link
Copy Markdown
Member

@carlossanlop This one's ready for release/8.0

@carlossanlop
carlossanlop merged commit 9c422ed into release/8.0Aug 31, 2023
@carlossanlop
carlossanlop deleted the backport/pr-91286-to-release/8.0 branch August 31, 2023 22:26
@radicalradical mentioned this pull request Sep 26, 2023
@ghostghost locked as resolved and limited conversation to collaborators Oct 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@jeffhandley@carlossanlop@stephentoub@tarekgh