Skip to content
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

Enable require-jsdoc lint and add two lints related to React #6403

Merged
merged 22 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
cc62b69
Add `require-jsdoc` lint, fix eslint errors, convert arrow functions …
somebody1234 Apr 24, 2023
2242c69
Fix lints
somebody1234 Apr 24, 2023
a3db9bc
Bump prettier version; run prettier
somebody1234 Apr 25, 2023
20c26d9
Address review
somebody1234 Apr 27, 2023
7273564
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 Apr 27, 2023
9a00bc0
Fix lint errors; increase coverage of `require-jsdoc` lint; add more …
somebody1234 Apr 27, 2023
9c15ba0
Add sections
somebody1234 Apr 28, 2023
f8e5773
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 Apr 28, 2023
0151739
Get rid of newlines adjacent to `/**` and `*/`
somebody1234 May 1, 2023
00874dd
Make all non-JSX lines under 100 characters long
somebody1234 May 1, 2023
a4e3858
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 3, 2023
68d9862
Fix lints
somebody1234 May 3, 2023
d577733
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 4, 2023
0f0230e
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 8, 2023
043ddb0
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 9, 2023
4fb4bda
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 10, 2023
bab7a2f
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 11, 2023
4e91bec
Fix lint errors and comment formatting
somebody1234 May 11, 2023
1e0aa42
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 12, 2023
de9ec38
Merge branch 'develop' into wip/sb/require-jsdoc
somebody1234 May 19, 2023
938cd82
Fix lint errors; minor refactor
somebody1234 May 19, 2023
cdd39f0
Minor fixes
somebody1234 May 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions app/ide-desktop/eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ import tsEslint from '@typescript-eslint/eslint-plugin'
import tsEslintParser from '@typescript-eslint/parser'
/* eslint-enable no-restricted-syntax */

// =================
// === Constants ===
// =================

const DIR_NAME = path.dirname(url.fileURLToPath(import.meta.url))
const NAME = 'enso'
/** An explicit whitelist of CommonJS modules, which do not support namespace imports.
Expand All @@ -35,7 +39,10 @@ const NOT_PASCAL_CASE = '/^(?!_?([A-Z][a-z0-9]*)+$)/'
const NOT_CAMEL_CASE = '/^(?!_?[a-z][a-z0-9*]*([A-Z0-9][a-z0-9]*)*$)/'
const WHITELISTED_CONSTANTS = 'logger|.+Context'
const NOT_CONSTANT_CASE = `/^(?!${WHITELISTED_CONSTANTS}$|_?[A-Z][A-Z0-9]*(_[A-Z0-9]+)*$)/`
const WITH_ROUTER = 'CallExpression[callee.name=withRouter]'

// =======================================
// === Restricted syntactic constructs ===
// =======================================

// Extracted to a variable because it needs to be used twice:
// - once as-is for `.d.ts`
Expand Down Expand Up @@ -113,7 +120,7 @@ const RESTRICTED_SYNTAXES = [
},
{
// Matches functions and arrow functions, but not methods.
selector: `:matches(FunctionDeclaration[id.name=${NOT_PASCAL_CASE}]:has(${JSX}), VariableDeclarator[id.name=${NOT_PASCAL_CASE}]:has(:matches(ArrowFunctionExpression.init ${JSX}, ${WITH_ROUTER})))`,
selector: `:matches(FunctionDeclaration[id.name=${NOT_PASCAL_CASE}]:has(${JSX}), VariableDeclarator[id.name=${NOT_PASCAL_CASE}]:has(:matches(ArrowFunctionExpression.init ${JSX})))`,
message: 'Use `PascalCase` for React components',
},
{
Expand All @@ -123,7 +130,7 @@ const RESTRICTED_SYNTAXES = [
},
{
// Matches non-functions.
selector: `:matches(Program, ExportNamedDeclaration, TSModuleBlock) > VariableDeclaration[kind=const] > VariableDeclarator[id.name=${NOT_CONSTANT_CASE}]:not(:has(:matches(ArrowFunctionExpression, ${WITH_ROUTER})))`,
selector: `:matches(Program, ExportNamedDeclaration, TSModuleBlock) > VariableDeclaration[kind=const] > VariableDeclarator[id.name=${NOT_CONSTANT_CASE}]:not(:has(:matches(ArrowFunctionExpression)))`,
message: 'Use `CONSTANT_CASE` for top-level constants that are not functions',
},
{
Expand Down Expand Up @@ -198,12 +205,28 @@ const RESTRICTED_SYNTAXES = [
'TSAsExpression:has(TSUnknownKeyword, TSNeverKeyword, TSAnyKeyword) > TSAsExpression',
message: 'Use type assertions to specific types instead of `unknown`, `any` or `never`',
},
{
selector: ':matches(MethodDeclaration, FunctionDeclaration) FunctionDeclaration',
message: 'Use arrow functions for nested functions',
},
{
selector: ':not(ExportNamedDeclaration) > TSInterfaceDeclaration[id.name=/Props$/]',
message: 'All React component `Props` types must be exported',
},
{
selector: 'FunctionDeclaration:has(:matches(ObjectPattern.params, ArrayPattern.params))',
message: 'Destructure function parameters in the body instead of in the parameter list',
},
{
selector: 'IfStatement > ExpressionStatement',
message: 'Wrap `if` branches in `{}`',
},
]

// ============================
// === ESLint configuration ===
// ============================

/* eslint-disable @typescript-eslint/naming-convention */
export default [
eslintJs.configs.recommended,
Expand All @@ -230,6 +253,26 @@ export default [
...tsEslint.configs['recommended-requiring-type-checking']?.rules,
...tsEslint.configs.strict?.rules,
eqeqeq: ['error', 'always', { null: 'never' }],
'jsdoc/require-jsdoc': [
'error',
{
require: {
FunctionDeclaration: true,
MethodDefinition: true,
ClassDeclaration: true,
ArrowFunctionExpression: false,
FunctionExpression: true,
},
// Top-level constants should require JSDoc as well,
// however it does not seem like there is a way to do this.
contexts: [
'TSInterfaceDeclaration',
'TSEnumDeclaration',
'TSTypeAliasDeclaration',
'TSMethodSignature',
],
},
],
'sort-imports': ['error', { allowSeparatedGroups: true }],
'no-restricted-syntax': ['error', ...RESTRICTED_SYNTAXES],
'prefer-arrow-callback': 'error',
Expand Down
9 changes: 6 additions & 3 deletions app/ide-desktop/lib/client/dist.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
/**
* @file This script creates a packaged IDE distribution using electron-builder.
/** @file This script creates a packaged IDE distribution using electron-builder.
* Behaviour details are controlled by the environment variables or CLI arguments.
* @see Arguments
* @see electronBuilderConfig.Arguments
*/

import * as electronBuilderConfig from './electron-builder-config'

// ==============================
// === Build Electron package ===
// ==============================

await electronBuilderConfig.buildPackage(electronBuilderConfig.args)
63 changes: 41 additions & 22 deletions app/ide-desktop/lib/client/electron-builder-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ import signArchivesMacOs from './tasks/signArchivesMacOs'

import BUILD_INFO from '../../build.json' assert { type: 'json' }

// =============
// === Types ===
// =============

/** The parts of the electron-builder configuration that we want to keep configurable.
*
* @see `args` definition below for fields description.
*/
* @see `args` definition below for fields description. */
export interface Arguments {
// This is returned by a third-party library we do not control.
// eslint-disable-next-line no-restricted-syntax
Expand All @@ -38,7 +41,12 @@ export interface Arguments {
platform: electronBuilder.Platform
}

/** CLI argument parser (with support for environment variables) that provides the necessary options. */
//======================================
// === Argument parser configuration ===
//======================================

/** CLI argument parser (with support for environment variables) that provides
* the necessary options. */
export const args: Arguments = await yargs(process.argv.slice(2))
.env('ENSO_BUILD')
.option({
Expand Down Expand Up @@ -79,6 +87,10 @@ export const args: Arguments = await yargs(process.argv.slice(2))
},
}).argv

// ======================================
// === Electron builder configuration ===
// ======================================

/** Based on the given arguments, creates a configuration for the Electron Builder. */
export function createElectronBuilderConfig(passedArgs: Arguments): electronBuilder.Configuration {
return {
Expand All @@ -91,8 +103,9 @@ export function createElectronBuilderConfig(passedArgs: Arguments): electronBuil
artifactName: 'enso-${os}-${version}.${ext}',
/** Definitions of URL {@link electronBuilder.Protocol} schemes used by the IDE.
*
* Electron will register all URL protocol schemes defined here with the OS. Once a URL protocol
* scheme is registered with the OS, any links using that scheme will function as "deep links".
* Electron will register all URL protocol schemes defined here with the OS.
* Once a URL protocol scheme is registered with the OS, any links using that scheme
* will function as "deep links".
* Deep links are used to redirect the user from external sources (e.g., system web browser,
* email client) to the IDE.
*
Expand All @@ -104,15 +117,16 @@ export function createElectronBuilderConfig(passedArgs: Arguments): electronBuil
* For details on how this works, see:
* https://www.electronjs.org/docs/latest/tutorial/launch-app-from-url-in-another-app. */
protocols: [
/** Electron URL protocol scheme definition for deep links to authentication flow pages. */
/** Electron URL protocol scheme definition for deep links to authentication pages. */
{
name: `${common.PRODUCT_NAME} url`,
schemes: [common.DEEP_LINK_SCHEME],
role: 'Editor',
},
],
mac: {
// We do not use compression as the build time is huge and file size saving is almost zero.
// Compression is not used as the build time is huge and file size saving
// almost zero.
// This type assertion is UNSAFE, and any users MUST verify that
// they are passing a valid value to `target`.
// eslint-disable-next-line no-restricted-syntax
Expand All @@ -127,18 +141,20 @@ export function createElectronBuilderConfig(passedArgs: Arguments): electronBuil
// This is a custom check that is not working correctly, so we disable it. See for more
// details https://kilianvalkhof.com/2019/electron/notarizing-your-electron-application/
gatekeeperAssess: false,
// Location of the entitlements files with the entitlements we need to run our application
// in the hardened runtime.
// Location of the entitlements files with the entitlements we need to run
// our application in the hardened runtime.
entitlements: './entitlements.mac.plist',
entitlementsInherit: './entitlements.mac.plist',
},
win: {
// We do not use compression as the build time is huge and file size saving is almost zero.
// Compression is not used as the build time is huge and file size saving
// almost zero.
target: passedArgs.target ?? 'nsis',
icon: `${passedArgs.iconsDist}/icon.ico`,
},
linux: {
// We do not use compression as the build time is huge and file size saving is almost zero.
// Compression is not used as the build time is huge and file size saving
// is almost zero.
target: passedArgs.target ?? 'AppImage',
icon: `${passedArgs.iconsDist}/png`,
category: 'Development',
Expand Down Expand Up @@ -231,12 +247,14 @@ export function createElectronBuilderConfig(passedArgs: Arguments): electronBuil
})

console.log(' • Notarizing.')
// The type-cast is safe because this is only executes
// when `platform === electronBuilder.Platform.MAC`.
// eslint-disable-next-line no-restricted-syntax
const macBuildOptions = buildOptions as macOptions.MacConfiguration
await electronNotarize.notarize({
// This will always be defined since we set it at the top of this object.
// The type-cast is safe because this is only executes
// when `platform === electronBuilder.Platform.MAC`.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion, no-restricted-syntax
appBundleId: (buildOptions as macOptions.MacConfiguration).appId!,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
appBundleId: macBuildOptions.appId!,
appPath: `${appOutDir}/${appName}.app`,
appleId: process.env.APPLEID,
appleIdPassword: process.env.APPLEIDPASS,
Expand All @@ -250,12 +268,13 @@ export function createElectronBuilderConfig(passedArgs: Arguments): electronBuil

/** Build the IDE package with Electron Builder. */
export async function buildPackage(passedArgs: Arguments) {
// `electron-builder` checks for presence of `node_modules` directory. If it is not present, it will
// install dependencies with `--production` flag (erasing all dev-only dependencies). This does not
// work sensibly with NPM workspaces. We have our `node_modules` in the root directory, not here.
// `electron-builder` checks for presence of `node_modules` directory. If it is not present, it
// will install dependencies with the`--production` flag(erasing all dev - only dependencies).
// This does not work sensibly with NPM workspaces. We have our `node_modules` in
// the root directory, not here.
//
// Without this workaround, `electron-builder` will end up erasing its own dependencies and failing
// because of that.
// Without this workaround, `electron-builder` will end up erasing its own dependencies and
// failing because of that.
await fs.mkdir('node_modules', { recursive: true })

const cliOpts: electronBuilder.CliOptions = {
Expand All @@ -266,7 +285,7 @@ export async function buildPackage(passedArgs: Arguments) {
const result = await electronBuilder.build(cliOpts)
console.log('Electron Builder is done. Result:', result)
// FIXME: https://github.com/enso-org/enso/issues/6082
// This is workaround which fixes esbuild freezing after successfully finishing the electronBuilder.build.
// It's safe to exit(0) since all processes are finished.
// This is a workaround which fixes esbuild hanging after successfully finishing
// `electronBuilder.build`. It is safe to `exit(0)` since all processes are finished.
process.exit(0)
}
9 changes: 5 additions & 4 deletions app/ide-desktop/lib/client/esbuild-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@ import * as paths from './paths'
// === Bundling ===
// ================

/**
* Get the bundler options using the environment.
/** Get the bundler options using the environment.
*
* The following environment variables are required:
* - `ENSO_BUILD_IDE` - output directory for bundled client files;
* - `ENSO_BUILD_PROJECT_MANAGER_IN_BUNDLE_PATH` - path to the project manager executable relative to the PM bundle root;
* - `ENSO_BUILD_IDE_BUNDLED_ENGINE_VERSION` - version of the Engine (backend) that is bundled along with this client build.
* - `ENSO_BUILD_PROJECT_MANAGER_IN_BUNDLE_PATH` - path to the project manager executable relative
* to the PM bundle root;
* - `ENSO_BUILD_IDE_BUNDLED_ENGINE_VERSION` - version of the Engine (backend) that is bundled
* along with this client build.
*
* @see bundlerOptions
*/
Expand Down
4 changes: 4 additions & 0 deletions app/ide-desktop/lib/client/file-associations.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
/** @file File associations for client application. */

// =========================
// === File associations ===
// =========================

/** The extension for the source file, without the leading period character. */
export const SOURCE_FILE_EXTENSION = 'enso'

Expand Down
2 changes: 1 addition & 1 deletion app/ide-desktop/lib/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"yargs": "17.6.2"
},
"comments": {
"electron-builder": "We cannot update it to never version because of NSIS installer issue: https://github.com/enso-org/enso/issues/5169"
"electron-builder": "Cannot be updated to a newer version because of a NSIS installer issue: https://github.com/enso-org/enso/issues/5169"
},
"devDependencies": {
"crypto-js": "4.1.1",
Expand Down
7 changes: 6 additions & 1 deletion app/ide-desktop/lib/client/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

import * as utils from '../../utils'

/** Path to the Project Manager bundle within the electron distribution (relative to the electron's resources directory). */
// ==========================
// === Paths to resources ===
// ==========================

/** Path to the Project Manager bundle within the electron distribution
* (relative to electron's resources directory). */
export const PROJECT_MANAGER_BUNDLE = 'enso'

/** Distribution directory for IDE. */
Expand Down
38 changes: 19 additions & 19 deletions app/ide-desktop/lib/client/src/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const logger = contentConfig.logger
// === Initialize Authentication Module ===
// ========================================

/** Configures all the functionality that must be set up in the Electron app to support
/** Configure all the functionality that must be set up in the Electron app to support
* authentication-related flows. Must be called in the Electron app `whenReady` event.
*
* @param window - A function that returns the main Electron window. This argument is a lambda and
Expand All @@ -104,7 +104,7 @@ export function initModule(window: () => electron.BrowserWindow) {
initSaveAccessTokenListener()
}

/** Registers an Inter-Process Communication (IPC) channel between the Electron application and the
/** Register an Inter-Process Communication (IPC) channel between the Electron application and the
* served website.
*
* This channel listens for {@link ipc.Channel.openUrlInSystemBrowser} events. When this kind of
Expand All @@ -122,7 +122,7 @@ function initIpc() {
})
}

/** Registers a listener that fires a callback for `open-url` events, when the URL is a deep link.
/** Register a listener that fires a callback for `open-url` events, when the URL is a deep link.
*
* This listener is used to open a page in *this* application window, when the user is
* redirected to a URL with a protocol supported by this application.
Expand All @@ -135,13 +135,11 @@ function initOpenUrlListener(window: () => electron.BrowserWindow) {
})
}

/**
* Handles the 'open-url' event by parsing the received URL, checking if it is a deep link, and
/** Handle the 'open-url' event by parsing the received URL, checking if it is a deep link, and
* sending it to the appropriate BrowserWindow via IPC.
*
* @param url - The URL to handle.
* @param window - A function that returns the BrowserWindow to send the parsed URL to.
*/
* @param window - A function that returns the BrowserWindow to send the parsed URL to. */
export function onOpenUrl(url: URL, window: () => electron.BrowserWindow) {
logger.log(`Received 'open-url' event for '${url.toString()}'.`)
if (url.protocol !== `${common.DEEP_LINK_SCHEME}:`) {
Expand All @@ -152,30 +150,32 @@ export function onOpenUrl(url: URL, window: () => electron.BrowserWindow) {
}
}

/** Registers a listener that fires a callback for `save-access-token` events.
/** Register a listener that fires a callback for `save-access-token` events.
*
* This listener is used to save given access token to credentials file to be later used by enso backend.
* This listener is used to save given access token to credentials file to be later used by
* the backend.
*
* Credentials file is placed in users home directory in `.enso` subdirectory in `credentials` file. */
* The credentials file is placed in the user's home directory in the `.enso` subdirectory
* in the `credentials` file. */
function initSaveAccessTokenListener() {
electron.ipcMain.on(ipc.Channel.saveAccessToken, (event, accessToken: string) => {
/** Enso home directory for credentials file. */
const ensoCredentialsDirectoryName = '.enso'
/** Enso credentials file. */
const ensoCredentialsFileName = 'credentials'
/** Home directory for the credentials file. */
const credentialsDirectoryName = `.${common.PRODUCT_NAME.toLowerCase()}`
/** File name of the credentials file. */
const credentialsFileName = 'credentials'
/** System agnostic credentials directory home path. */
const ensoCredentialsHomePath = path.join(os.homedir(), ensoCredentialsDirectoryName)
const credentialsHomePath = path.join(os.homedir(), credentialsDirectoryName)

fs.mkdir(ensoCredentialsHomePath, { recursive: true }, error => {
fs.mkdir(credentialsHomePath, { recursive: true }, error => {
if (error) {
logger.error(`Couldn't create ${ensoCredentialsDirectoryName} directory.`)
logger.error(`Couldn't create ${credentialsDirectoryName} directory.`)
} else {
fs.writeFile(
path.join(ensoCredentialsHomePath, ensoCredentialsFileName),
path.join(credentialsHomePath, credentialsFileName),
accessToken,
innerError => {
if (innerError) {
logger.error(`Could not write to ${ensoCredentialsFileName} file.`)
logger.error(`Could not write to ${credentialsFileName} file.`)
}
}
)
Expand Down
Loading