Rule S2077: support for additional database libraries: PetaPoco #6192
Conversation
martin-strecker-sonarsource
force-pushed
the
Martin/S2077_PetaPoco
branch
from
1576c5a
to
2465119
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
SonarCloud Quality Gate failed. |
| database.Query<Entity>(query, param + otherParam); // Noncompliant FP. The second argument is params object[] args and is safe. | ||
| database.Query<Entity>(new[] { typeof(Entity) }, null, query + param); // FN. This overload is not supported (sql string in the third parameter). | ||
| database.Query<Entity, Entity>(query + param); // Noncompliant | ||
| database.Query<Entity, Entity, Entity>(query + param); // Noncompliant | ||
| database.Query<Entity, Entity, Entity, Entity>(query + param); // Noncompliant | ||
| database.Query<Entity, Entity, Entity, Entity, Entity>(query + param); // Noncompliant | ||
|
|
||
| database.Fetch<Entity>(query); // Compliant | ||
| database.Fetch<Entity>(query + param); // Noncompliant | ||
| database.Fetch<Entity>(query + param); // Noncompliant | ||
| database.Fetch<Entity>(2, 10, query + param); // FN. This overload is not supported (sql string in the third parameter). | ||
| database.Fetch<Entity>(param + otherParam, 10, query); // Noncompliant FP. The first argument is not a string. |
There was a problem hiding this comment.
@pavel-mikula-sonarsource I stopped here, for now, to get some feedback from you. This library has more than 20 overloads for each of Query, Fetch, QueryAsync, and FetchAsync. The SQL string parameter can appear on positions 1, 2, 3, and 4. Our current logic does only support 2 locations and increasing the count, even more, leads to more FPs because we would detect more violations in the params object[] args arguments.
I'm not sure how to proceed with this library but the current approach seems to be a dead end to me with too many FPs and FNs. A viable solution would use the argument-to-parameter mapping logic and use the parameter's name for detection (We could also improve detection for the other libraries, because these only support positional arguments but not named arguments at the moment).
Here is the overload where SQL is in the fourth position:
await database.FetchAsync<Entity>(CancellationToken.None, page: 1, itemsPerPage: 10, "Sql");
github-actions
bot
assigned pavel-mikula-sonarsource and unassigned martin-strecker-sonarsource
pavel-mikula-sonarsource
moved this from Review in progress
to In progress
in Best Kanban
pavel-mikula-sonarsource
assigned martin-strecker-sonarsource and unassigned pavel-mikula-sonarsource
Part of #3905