Skip to content

Commit ed9859a

Browse files
[Security solutions] Adds linter rule to forbid usage of no-non-null-assertion (TypeScript ! bang operator) (elastic#114375)
## Summary Fixes: elastic#114535 **What this linter rule does:** * Sets the [@typescript-eslint/no-non-null-assertion](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-non-null-assertion.md) linter rule to become an error if seen. If you try to use the `!` operator you get an error and nice helper message that tries to encourage better practices such as this one: <img width="1635" alt="Screen Shot 2021-10-07 at 11 26 14 AM" src="https://user-images.githubusercontent.com/1151048/136474207-f38d3461-0af9-4cdc-885b-632cb49d8a24.png"> **Why are we deciding to set this linter rule?** * Recommended from Kibana [styleguide](https://github.com/elastic/kibana/blob/master/STYLEGUIDE.mdx#avoid-non-null-assertions) for ~2 years now and still recommended. * A lot of TypeScript has evolved and has operators such as `?` which can replace the `!` in most cases. Other cases can use a throw explicitly or other ways to manage this. * Some types can change instead of using this operator and we should just change the types. * TypeScript flows have improved and when we upgrade the linter will cause errors where we can remove the `!` operator which is 👍 better than leaving them when they're not needed anymore. * Newer programmers and team members sometimes mistake it for the `?` when it is not the same thing. * We have had past bugs and bugs recently because of these. * It's easier to use the linter to find bugs than to rely on manual tests. **How did Frank fix all the 403 areas in which the linter goes off?** * Anywhere I could remove the `!` operator without side effects or type script errors I just removed the `!` operator. * Anywhere in test code where I could replace the code with a `?` or a `throw` I went through that route. * Within the code areas (non test code) where I saw what looks like a simple bug that I could fix using a `?? []` or `?` operator or `String(value)` or fixing a simple type I would choose that route first. These areas I marked in the code review with the words `NOTE:` for anyone to look at. * Within all other areas of the code and tests where anything looked more involved I just disabled the linter for that particular line. When in doubt I chose this route. ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
1 parent 45f02d3 commit ed9859a

File tree

149 files changed

+367
-238
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

149 files changed

+367
-238
lines changed

.eslintrc.js

+35-3
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,12 @@ module.exports = {
899899
},
900900

901901
/**
902-
* Security Solution overrides
902+
* Security Solution overrides. These rules below are maintained and owned by
903+
* the people within the security-solution-platform team. Please see ping them
904+
* or check with them if you are encountering issues, have suggestions, or would
905+
* like to add, change, or remove any particular rule. Linters, Typescript, and rules
906+
* evolve and change over time just like coding styles, so please do not hesitate to
907+
* reach out.
903908
*/
904909
{
905910
// front end and common typescript and javascript files only
@@ -922,6 +927,22 @@ module.exports = {
922927
],
923928
},
924929
},
930+
{
931+
// typescript only for front and back end, but excludes the test files.
932+
// We use this section to add rules in which we do not want to apply to test files.
933+
// This should be a very small set as most linter rules are useful for tests as well.
934+
files: [
935+
'x-pack/plugins/security_solution/**/*.{ts,tsx}',
936+
'x-pack/plugins/timelines/**/*.{ts,tsx}',
937+
],
938+
excludedFiles: [
939+
'x-pack/plugins/security_solution/**/*.{test,mock,test_helper}.{ts,tsx}',
940+
'x-pack/plugins/timelines/**/*.{test,mock,test_helper}.{ts,tsx}',
941+
],
942+
rules: {
943+
'@typescript-eslint/no-non-null-assertion': 'error',
944+
},
945+
},
925946
{
926947
// typescript only for front and back end
927948
files: [
@@ -1040,7 +1061,12 @@ module.exports = {
10401061
},
10411062

10421063
/**
1043-
* Lists overrides
1064+
* Lists overrides. These rules below are maintained and owned by
1065+
* the people within the security-solution-platform team. Please see ping them
1066+
* or check with them if you are encountering issues, have suggestions, or would
1067+
* like to add, change, or remove any particular rule. Linters, Typescript, and rules
1068+
* evolve and change over time just like coding styles, so please do not hesitate to
1069+
* reach out.
10441070
*/
10451071
{
10461072
// front end and common typescript and javascript files only
@@ -1215,8 +1241,14 @@ module.exports = {
12151241
],
12161242
},
12171243
},
1244+
12181245
/**
1219-
* Metrics entities overrides
1246+
* Metrics entities overrides. These rules below are maintained and owned by
1247+
* the people within the security-solution-platform team. Please see ping them
1248+
* or check with them if you are encountering issues, have suggestions, or would
1249+
* like to add, change, or remove any particular rule. Linters, Typescript, and rules
1250+
* evolve and change over time just like coding styles, so please do not hesitate to
1251+
* reach out.
12201252
*/
12211253
{
12221254
// front end and common typescript and javascript files only

x-pack/plugins/security_solution/common/detection_engine/utils.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ export const normalizeThresholdField = (
5151
? thresholdField
5252
: isEmpty(thresholdField)
5353
? []
54-
: [thresholdField!];
54+
: // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
55+
[thresholdField!];
5556
};
5657

5758
export const normalizeThresholdObject = (threshold: Threshold): ThresholdNormalized => {

x-pack/plugins/security_solution/common/endpoint/data_loaders/index_endpoint_hosts.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ export async function indexEndpointHostDocs({
161161
const indexedAgentResponse = await indexFleetAgentForHost(
162162
client,
163163
kbnClient,
164-
hostMetadata!,
164+
hostMetadata,
165165
realPolicies[appliedPolicyId].policy_id,
166166
kibanaVersion
167167
);

x-pack/plugins/security_solution/common/endpoint/data_loaders/index_fleet_agent.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export const indexFleetAgentForHost = async (
7373
.index<FleetServerAgent>({
7474
index: agentDoc._index,
7575
id: agentDoc._id,
76-
body: agentDoc._source!,
76+
body: agentDoc._source,
7777
op_type: 'create',
7878
})
7979
.catch(wrapErrorAndRejectPromise);

x-pack/plugins/security_solution/cypress/integration/cases/connectors.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -89,11 +89,11 @@ describe('Cases connectors', () => {
8989
addServiceNowConnector(snConnector);
9090

9191
cy.wait('@createConnector').then(({ response }) => {
92-
cy.wrap(response!.statusCode).should('eql', 200);
92+
cy.wrap(response?.statusCode).should('eql', 200);
9393
cy.get(TOASTER).should('have.text', "Created 'New connector'");
9494
cy.get(TOASTER).should('not.exist');
9595

96-
selectLastConnectorCreated(response!.body.id);
96+
selectLastConnectorCreated(response?.body.id);
9797

9898
cy.wait('@saveConnector', { timeout: 10000 }).its('response.statusCode').should('eql', 200);
9999
cy.get(SERVICE_NOW_MAPPING).first().should('have.text', 'short_description');

x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -351,10 +351,10 @@ describe('Custom detection rules deletion and edition', () => {
351351
goToRuleDetails();
352352

353353
cy.wait('@fetchRuleDetails').then(({ response }) => {
354-
cy.wrap(response!.statusCode).should('eql', 200);
354+
cy.wrap(response?.statusCode).should('eql', 200);
355355

356-
cy.wrap(response!.body.max_signals).should('eql', getExistingRule().maxSignals);
357-
cy.wrap(response!.body.enabled).should('eql', false);
356+
cy.wrap(response?.body.max_signals).should('eql', getExistingRule().maxSignals);
357+
cy.wrap(response?.body.enabled).should('eql', false);
358358
});
359359
});
360360

@@ -414,9 +414,9 @@ describe('Custom detection rules deletion and edition', () => {
414414
saveEditedRule();
415415

416416
cy.wait('@getRule').then(({ response }) => {
417-
cy.wrap(response!.statusCode).should('eql', 200);
417+
cy.wrap(response?.statusCode).should('eql', 200);
418418
// ensure that editing rule does not modify max_signals
419-
cy.wrap(response!.body.max_signals).should('eql', getExistingRule().maxSignals);
419+
cy.wrap(response?.body.max_signals).should('eql', getExistingRule().maxSignals);
420420
});
421421

422422
cy.get(RULE_NAME_HEADER).should('contain', `${getEditedRule().name}`);

x-pack/plugins/security_solution/cypress/integration/detection_rules/export_rule.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('Export rules', () => {
3535
goToManageAlertsDetectionRules();
3636
exportFirstRule();
3737
cy.wait('@export').then(({ response }) => {
38-
cy.wrap(response!.body).should('eql', expectedExportedRule(this.ruleResponse));
38+
cy.wrap(response?.body).should('eql', expectedExportedRule(this.ruleResponse));
3939
});
4040
});
4141
});

x-pack/plugins/security_solution/cypress/integration/timeline_templates/creation.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ describe('Timeline Templates', () => {
7070
addNameToTimeline(getTimeline().title);
7171

7272
cy.wait('@timeline').then(({ response }) => {
73-
const timelineId = response!.body.data.persistTimeline.timeline.savedObjectId;
73+
const timelineId = response?.body.data.persistTimeline.timeline.savedObjectId;
7474

7575
addDescriptionToTimeline(getTimeline().description);
7676
addNotesToTimeline(getTimeline().notes);

x-pack/plugins/security_solution/cypress/integration/timeline_templates/export.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ describe('Export timelines', () => {
3535
exportTimeline(this.templateId);
3636

3737
cy.wait('@export').then(({ response }) => {
38-
cy.wrap(response!.statusCode).should('eql', 200);
38+
cy.wrap(response?.statusCode).should('eql', 200);
3939

40-
cy.wrap(response!.body).should(
40+
cy.wrap(response?.body).should(
4141
'eql',
4242
expectedExportedTimelineTemplate(this.templateResponse)
4343
);

x-pack/plugins/security_solution/cypress/integration/timelines/export.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@ describe('Export timelines', () => {
3333
exportTimeline(this.timelineId);
3434

3535
cy.wait('@export').then(({ response }) => {
36-
cy.wrap(response!.statusCode).should('eql', 200);
36+
cy.wrap(response?.statusCode).should('eql', 200);
3737

38-
cy.wrap(response!.body).should('eql', expectedExportedTimeline(this.timelineResponse));
38+
cy.wrap(response?.body).should('eql', expectedExportedTimeline(this.timelineResponse));
3939
});
4040
});
4141
});

x-pack/plugins/security_solution/cypress/integration/timelines/search_or_filter.spec.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,17 @@ describe('Update kqlMode for timeline', () => {
5454
it('should be able to update timeline kqlMode with filter', () => {
5555
cy.get(TIMELINE_KQLMODE_FILTER).click();
5656
cy.wait('@update').then(({ response }) => {
57-
cy.wrap(response!.statusCode).should('eql', 200);
58-
cy.wrap(response!.body.data.persistTimeline.timeline.kqlMode).should('eql', 'filter');
57+
cy.wrap(response?.statusCode).should('eql', 200);
58+
cy.wrap(response?.body.data.persistTimeline.timeline.kqlMode).should('eql', 'filter');
5959
cy.get(ADD_FILTER).should('exist');
6060
});
6161
});
6262

6363
it('should be able to update timeline kqlMode with search', () => {
6464
cy.get(TIMELINE_KQLMODE_SEARCH).click();
6565
cy.wait('@update').then(({ response }) => {
66-
cy.wrap(response!.statusCode).should('eql', 200);
67-
cy.wrap(response!.body.data.persistTimeline.timeline.kqlMode).should('eql', 'search');
66+
cy.wrap(response?.statusCode).should('eql', 200);
67+
cy.wrap(response?.body.data.persistTimeline.timeline.kqlMode).should('eql', 'search');
6868
cy.get(ADD_FILTER).should('not.exist');
6969
});
7070
});

x-pack/plugins/security_solution/cypress/integration/urls/state.spec.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,8 @@ describe('url state', () => {
250250

251251
cy.wait('@timeline').then(({ response }) => {
252252
closeTimeline();
253-
cy.wrap(response!.statusCode).should('eql', 200);
254-
const timelineId = response!.body.data.persistTimeline.timeline.savedObjectId;
253+
cy.wrap(response?.statusCode).should('eql', 200);
254+
const timelineId = response?.body.data.persistTimeline.timeline.savedObjectId;
255255
cy.visit('/app/home');
256256
cy.visit(`/app/security/timelines?timeline=(id:'${timelineId}',isOpen:!t)`);
257257
cy.get(DATE_PICKER_APPLY_BUTTON_TIMELINE).should('exist');

x-pack/plugins/security_solution/cypress/integration/value_lists/value_lists.spec.ts

+7-7
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,8 @@ describe('value lists', () => {
174174
cy.wait('@exportList').then(({ response }) => {
175175
cy.fixture(listName).then((list: string) => {
176176
const [lineOne, lineTwo] = list.split('\n');
177-
expect(response!.body).to.contain(lineOne);
178-
expect(response!.body).to.contain(lineTwo);
177+
expect(response?.body).to.contain(lineOne);
178+
expect(response?.body).to.contain(lineTwo);
179179
});
180180
});
181181
});
@@ -189,8 +189,8 @@ describe('value lists', () => {
189189
cy.wait('@exportList').then(({ response }) => {
190190
cy.fixture(listName).then((list: string) => {
191191
const [lineOne, lineTwo] = list.split('\n');
192-
expect(response!.body).to.contain(lineOne);
193-
expect(response!.body).to.contain(lineTwo);
192+
expect(response?.body).to.contain(lineOne);
193+
expect(response?.body).to.contain(lineTwo);
194194
});
195195
});
196196
});
@@ -204,8 +204,8 @@ describe('value lists', () => {
204204
cy.wait('@exportList').then(({ response }) => {
205205
cy.fixture(listName).then((list: string) => {
206206
const [lineOne, lineTwo] = list.split('\n');
207-
expect(response!.body).to.contain(lineOne);
208-
expect(response!.body).to.contain(lineTwo);
207+
expect(response?.body).to.contain(lineOne);
208+
expect(response?.body).to.contain(lineTwo);
209209
});
210210
});
211211
});
@@ -219,7 +219,7 @@ describe('value lists', () => {
219219
cy.wait('@exportList').then(({ response }) => {
220220
cy.fixture(listName).then((list: string) => {
221221
const [lineOne] = list.split('\n');
222-
expect(response!.body).to.contain(lineOne);
222+
expect(response?.body).to.contain(lineOne);
223223
});
224224
});
225225
});

x-pack/plugins/security_solution/cypress/screens/timelines.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ export const BULK_ACTIONS = '[data-test-subj="utility-bar-action-button"]';
99

1010
export const EXPORT_TIMELINE_ACTION = '[data-test-subj="export-timeline-action"]';
1111

12-
export const TIMELINE = (id: string) => {
12+
export const TIMELINE = (id: string | undefined) => {
13+
if (id == null) {
14+
throw new TypeError('id should never be null or undefined');
15+
}
1316
return `[data-test-subj="title-${id}"]`;
1417
};
1518

x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts

+7-4
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ export const fillDefineCustomRuleWithImportedQueryAndContinue = (
254254
rule: CustomRule | OverrideRule
255255
) => {
256256
cy.get(IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK).click();
257-
cy.get(TIMELINE(rule.timeline.id!)).click();
257+
cy.get(TIMELINE(rule.timeline.id)).click();
258258
cy.get(CUSTOM_QUERY_INPUT).should('have.value', rule.customQuery);
259259
cy.get(DEFINE_CONTINUE_BUTTON).should('exist').click({ force: true });
260260

@@ -273,7 +273,7 @@ export const fillDefineThresholdRule = (rule: ThresholdRule) => {
273273
const threshold = 1;
274274

275275
cy.get(IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK).click();
276-
cy.get(TIMELINE(rule.timeline.id!)).click();
276+
cy.get(TIMELINE(rule.timeline.id)).click();
277277
cy.get(COMBO_BOX_CLEAR_BTN).first().click();
278278

279279
rule.index.forEach((index) => {
@@ -298,7 +298,7 @@ export const fillDefineThresholdRuleAndContinue = (rule: ThresholdRule) => {
298298
cy.wrap($el).type(rule.thresholdField, { delay: 35 });
299299

300300
cy.get(IMPORT_QUERY_FROM_SAVED_TIMELINE_LINK).click();
301-
cy.get(TIMELINE(rule.timeline.id!)).click();
301+
cy.get(TIMELINE(rule.timeline.id)).click();
302302
cy.get(CUSTOM_QUERY_INPUT).should('have.value', rule.customQuery);
303303
cy.get(THRESHOLD_INPUT_AREA)
304304
.find(INPUT)
@@ -314,9 +314,12 @@ export const fillDefineThresholdRuleAndContinue = (rule: ThresholdRule) => {
314314
};
315315

316316
export const fillDefineEqlRuleAndContinue = (rule: CustomRule) => {
317+
if (rule.customQuery == null) {
318+
throw new TypeError('The rule custom query should never be undefined or null ');
319+
}
317320
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('exist');
318321
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible');
319-
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type(rule.customQuery!);
322+
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type(rule.customQuery);
320323
cy.get(RULES_CREATION_FORM).find(EQL_QUERY_VALIDATION_SPINNER).should('not.exist');
321324
cy.get(RULES_CREATION_PREVIEW)
322325
.find(QUERY_PREVIEW_BUTTON)

x-pack/plugins/security_solution/cypress/tasks/login.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ const LOGIN_API_ENDPOINT = '/internal/security/login';
5757
*/
5858
export const getUrlWithRoute = (role: ROLES, route: string) => {
5959
const url = Cypress.config().baseUrl;
60-
const kibana = new URL(url!);
60+
const kibana = new URL(String(url));
6161
const theUrl = `${Url.format({
6262
auth: `${role}:changeme`,
6363
username: role,
@@ -83,7 +83,7 @@ interface User {
8383
*/
8484
export const constructUrlWithUser = (user: User, route: string) => {
8585
const url = Cypress.config().baseUrl;
86-
const kibana = new URL(url!);
86+
const kibana = new URL(String(url));
8787
const hostname = kibana.hostname;
8888
const username = user.username;
8989
const password = user.password;

x-pack/plugins/security_solution/cypress/tasks/rule_details.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const activatesRule = () => {
3232
cy.get(RULE_SWITCH).should('be.visible');
3333
cy.get(RULE_SWITCH).click();
3434
cy.wait('@bulk_update').then(({ response }) => {
35-
cy.wrap(response!.statusCode).should('eql', 200);
35+
cy.wrap(response?.statusCode).should('eql', 200);
3636
});
3737
};
3838

x-pack/plugins/security_solution/public/common/components/charts/barchart.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ export const BarChartBaseComponent = ({
9494
yAccessors={yAccessors}
9595
timeZone={timeZone}
9696
splitSeriesAccessors={splitSeriesAccessors}
97-
data={series.value!}
97+
data={series.value ?? []}
9898
stackAccessors={get('configs.series.stackAccessors', chartConfigs)}
9999
color={series.color ? series.color : undefined}
100100
/>

x-pack/plugins/security_solution/public/common/components/event_details/helpers.test.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { mockBrowserFields } from '../../containers/source/mock';
1212

1313
const aField = {
1414
...mockDetailItemData[4],
15-
...mockBrowserFields.base.fields!['@timestamp'],
15+
...mockBrowserFields.base.fields?.['@timestamp'],
1616
};
1717

1818
describe('helpers', () => {

x-pack/plugins/security_solution/public/common/components/event_details/helpers.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export const getColumnsWithTimestamp = ({
127127
export const getExampleText = (example: string | number | null | undefined): string =>
128128
!isEmpty(example) ? `Example: ${example}` : '';
129129

130-
export const getIconFromType = (type: string | null) => {
130+
export const getIconFromType = (type: string | null | undefined) => {
131131
switch (type) {
132132
case 'string': // fall through
133133
case 'keyword':

x-pack/plugins/security_solution/public/common/components/events_viewer/events_viewer.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,7 @@ const EventsViewerComponent: React.FC<Props> = ({
236236
useTimelineEvents({
237237
docValueFields,
238238
fields,
239-
filterQuery: combinedQueries!.filterQuery,
239+
filterQuery: combinedQueries?.filterQuery,
240240
id,
241241
indexNames,
242242
limit: itemsPerPage,
@@ -300,7 +300,7 @@ const EventsViewerComponent: React.FC<Props> = ({
300300
height={headerFilterGroup ? COMPACT_HEADER_HEIGHT : EVENTS_VIEWER_HEADER_HEIGHT}
301301
subtitle={utilityBar ? undefined : subtitle}
302302
title={globalFullScreen ? titleWithExitFullScreen : justTitle}
303-
isInspectDisabled={combinedQueries!.filterQuery === undefined}
303+
isInspectDisabled={combinedQueries?.filterQuery === undefined}
304304
>
305305
{HeaderSectionContent}
306306
</HeaderSection>

0 commit comments

Comments
 (0)