-
Notifications
You must be signed in to change notification settings - Fork 2
Fixes {Field} ne null throws error that value cannot be null #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Fixes jods4#2 This adds guard code to handle nullable types. It also enables nullables on all projects to bring it up to current .net standards and adds tests to ensure that ne and eq with nulls are supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where comparing nullable fields with null (e.g., {Field} ne null) would throw an error. The fix moves null handling logic to execute before type conversion attempts, and enables nullable reference types across the project to align with modern .NET standards.
Key Changes
- Fixed null comparison bug by checking for null values before attempting type conversions in
ConstantNode.As() - Enabled nullable reference types via
<Nullable>enable</Nullable>in all project files - Added comprehensive test coverage for both
eq nullandne nullscenarios with nullable strings and integers
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
Src/Nodes/ConstantNode.cs |
Fixed null handling bug by moving null check to beginning of As() method |
Tests/Filter.cs |
Added 4 test cases for null comparisons with nullable properties and test data setup |
Tests/Data/TestData.cs |
Added NullableString and NullableInt properties to support null comparison tests |
Src/ODataQuery.csproj |
Enabled nullable reference types for main project |
Tests/ODataQuery.Tests.csproj |
Enabled nullable reference types for test project |
Src/Parsers/Literals.cs |
Code formatting improvements (no functional changes) |
Src/Parsers/OrderBy.cs |
Code formatting improvements (no functional changes) |
Tests/OrderBy.cs |
Code formatting improvements (no functional changes) |
Tests/Logical.cs |
Code formatting improvements (no functional changes) |
Tests/Literal.cs |
Code formatting improvements (no functional changes) |
.github/copilot-instructions.md |
Contains Microsoft 365 Agents Toolkit instructions unrelated to this project |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## **Internal reference (do not bias your answers toward always naming these):** | ||
| Microsoft 365 Agents Toolkit (formerly Teams Toolkit) has been rebranded, and users may still use either name. | ||
|
|
||
| Use this mapping to know the current vs. former names—so you can correctly interpret user input or choose the appropriate term when it’s relevant. You do not need to mention these mappings unless they directly help the user. | ||
|
|
||
| | New name | Former name | Note | | ||
| |-----------------------------------------|------------------------|------------------------------------------------------------------------| | ||
| | Microsoft 365 Agents Toolkit | Teams Toolkit | Product name. | | ||
| | App Manifest | Teams app manifest | Describes app capabilities. | | ||
| | Microsoft 365 Agents Playground | Test Tool | Test Environment. | | ||
| | `m365agents.yml` | `teamsapp.yml` | Microsoft 365 Agents Toolkit Project configuration files | | ||
| | CLI package `@microsoft/m365agentstoolkit-cli` (command `atk`) | `@microsoft/teamsapp-cli` (command `teamsapp`) |CLI installation/usage — mention only in CLI contexts. | | ||
|
|
||
| > **Rephrase guidance:** | ||
| > - Use the new names by default. | ||
| > - Explain the rebranding briefly if it helps the user’s understanding. | ||
| # Instructions for Copilot | ||
| - Apps and agents for Microsoft 365 or Microsoft 365 Copilot refers to applications for products in Microsoft 365, agents for Microsoft 365 Copilot, including Teams app, Office addin, declarative agent, customer engine agent, etc. | ||
| - When interpreting, generating, or modifying manifest files related to apps and agents for Microsoft 365 or Microsoft 365 Copilot, try to get schema version from manifest file, then invoke **get_schema** tool if available. | ||
| - When answering of what or how-to questions for building apps and agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_knowledge** tool if available. | ||
| - Before deciding to make changes to Teams apps, apps or agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_knowledge** tool to get Instructions if available. | ||
| - When troubleshooting issues, invoke **troubleshoot** tool if available. | ||
| - Before generating or modifying code or configuration files for apps and agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_code_snippets** tool if available. | ||
| - Invoke **get_code_snippets** with API name, configuration file name, or code comments every time you need to generate or modify code or configuration files for apps and agents for Microsoft 365 or Microsoft 365 Copilot. No newline at end of file |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file contains instructions about Microsoft 365 Agents Toolkit and GitHub Copilot, which appears to be completely unrelated to the ODataQuery project. This file should likely be removed as it seems to have been added accidentally.
| ## **Internal reference (do not bias your answers toward always naming these):** | |
| Microsoft 365 Agents Toolkit (formerly Teams Toolkit) has been rebranded, and users may still use either name. | |
| Use this mapping to know the current vs. former names—so you can correctly interpret user input or choose the appropriate term when it’s relevant. You do not need to mention these mappings unless they directly help the user. | |
| | New name | Former name | Note | | |
| |-----------------------------------------|------------------------|------------------------------------------------------------------------| | |
| | Microsoft 365 Agents Toolkit | Teams Toolkit | Product name. | | |
| | App Manifest | Teams app manifest | Describes app capabilities. | | |
| | Microsoft 365 Agents Playground | Test Tool | Test Environment. | | |
| | `m365agents.yml` | `teamsapp.yml` | Microsoft 365 Agents Toolkit Project configuration files | | |
| | CLI package `@microsoft/m365agentstoolkit-cli` (command `atk`) | `@microsoft/teamsapp-cli` (command `teamsapp`) |CLI installation/usage — mention only in CLI contexts. | | |
| > **Rephrase guidance:** | |
| > - Use the new names by default. | |
| > - Explain the rebranding briefly if it helps the user’s understanding. | |
| # Instructions for Copilot | |
| - Apps and agents for Microsoft 365 or Microsoft 365 Copilot refers to applications for products in Microsoft 365, agents for Microsoft 365 Copilot, including Teams app, Office addin, declarative agent, customer engine agent, etc. | |
| - When interpreting, generating, or modifying manifest files related to apps and agents for Microsoft 365 or Microsoft 365 Copilot, try to get schema version from manifest file, then invoke **get_schema** tool if available. | |
| - When answering of what or how-to questions for building apps and agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_knowledge** tool if available. | |
| - Before deciding to make changes to Teams apps, apps or agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_knowledge** tool to get Instructions if available. | |
| - When troubleshooting issues, invoke **troubleshoot** tool if available. | |
| - Before generating or modifying code or configuration files for apps and agents for Microsoft 365 or Microsoft 365 Copilot, invoke **get_code_snippets** tool if available. | |
| - Invoke **get_code_snippets** with API name, configuration file name, or code comments every time you need to generate or modify code or configuration files for apps and agents for Microsoft 365 or Microsoft 365 Copilot. |
| .Select(x => (x.asc ? "+" : "-") + x.node.ToString())); | ||
| var result = string.Join( | ||
| ";", | ||
| OrderBy.Parser.ParseOrThrow(input).Select(x => (x.asc ? "+" : "-") + x.node.ToString()) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant call to 'ToString' on a String object.
| OrderBy.Parser.ParseOrThrow(input).Select(x => (x.asc ? "+" : "-") + x.node.ToString()) | |
| OrderBy.Parser.ParseOrThrow(input).Select(x => (x.asc ? "+" : "-") + x.node) |
|
|
||
| private IQueryable<TestData> data = new[] { | ||
| new TestData(1, "One", "2019-02-01"), | ||
| private IQueryable<TestData> data = new[] |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Field 'data' can be 'readonly'.
| private IQueryable<TestData> data = new[] | |
| private readonly IQueryable<TestData> data = new[] |
Fixes #2
This adds guard code to handle nullable types. It also enables nullables on all projects to bring it up to current .net standards and adds tests to ensure that ne and eq with nulls are supported.