Make lint runnable, cover the shipped lib/, and run it in CI

.github/workflows/test.yml

@@ -8,6 +8,41 @@ on:
   workflow_dispatch:
 
 jobs:
+  # Lint runs as its own job, first and cheapest. It needs no SQL Server, no ODBC
+  # driver and no native build, so `npm ci --ignore-scripts` is enough and it
+  # finishes in well under a minute.
+  #
+  # It exists because `npm run lint` was broken for weeks without anyone
+  # noticing: the script was never wired into CI, so a TypeScript major bump that
+  # made typescript-eslint refuse to load cost nothing and went unseen. Lint that
+  # nothing runs is lint that rots.
+  lint:
+    name: Lint
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Create directory structure
+      run: mkdir -p node_modules
+
+    - uses: actions/checkout@v7
+      with:
+        path: node_modules/msnodesqlv8
+
+    - name: Setup Node.js
+      uses: actions/setup-node@v7
+      with:
+        node-version: 22
+
+    # --ignore-scripts skips the native build: eslint never loads the addon, and
+    # building it here would add minutes for no benefit.
+    - name: Install dependencies
+      working-directory: node_modules/msnodesqlv8
+      run: npm ci --ignore-scripts
+
+    - name: Lint
+      working-directory: node_modules/msnodesqlv8
+      run: npm run lint
+
   test-linux:
     name: Test Linux - Node ${{ matrix.node }}
     strategy:

eslint.config.mjs

@@ -85,12 +85,31 @@ export default [
       '@typescript-eslint/no-empty-object-type': 'off',
       '@typescript-eslint/no-empty-function': 'off',
       '@typescript-eslint/no-deprecated': 'off',
+      // Harness and sample code rethrows errors straight from driver
+      // callbacks. QueryCb types err as `Error | undefined`, but the
+      // overloaded query(sqlOrQuery, paramsOrCb?, cb?) signature leaves it
+      // unresolvable at some call sites, and test/env/cmd-test.ts declares
+      // several of its own handlers as `err: any`. The rule then cannot
+      // prove the thrown value is an Error even where a `!= null` guard
+      // precedes it. Off for the same reason the no-unsafe-* rules above
+      // are off - the driver's own surface is loosely typed.
+      '@typescript-eslint/only-throw-error': 'off',
+      // `res![0]` in a test that has just inserted the row it selects.
+      // Consistent with consistent-type-assertions being off above.
+      '@typescript-eslint/no-non-null-assertion': 'off',
       '@typescript-eslint/no-unnecessary-type-arguments': 'off',
       '@typescript-eslint/no-unnecessary-type-parameters': 'off',
       '@typescript-eslint/no-unnecessary-type-conversion': 'off',
       '@typescript-eslint/no-base-to-string': 'off',
       '@typescript-eslint/prefer-optional-chain': 'off',
       '@typescript-eslint/prefer-for-of': 'off',
+      // Its autofix rewrites Object.prototype.hasOwnProperty.call(o, k) to
+      // Object.hasOwn(o, k), which does not compile here: tsconfig sets
+      // target es6 with no explicit lib, and Object.hasOwn is es2022, so tsc
+      // fails with TS2550. Raising the target is the real fix but it changes
+      // emitted dist/ output and the generated declarations, so it wants its
+      // own change rather than riding along with lint wiring.
+      'prefer-object-has-own': 'off',
       'no-plusplus': 'off',
       'no-console': 'off',
       'no-param-reassign': 'off',
@@ -119,6 +138,39 @@ export default [
       '@eslint-community/eslint-comments/no-unlimited-disable': 'off'
     }
   },
+  {
+    // lib/ is the shipped JavaScript and had NO lint coverage at all: the block
+    // above targets lib/**/*.ts, and the only .ts in lib is index.d.ts, which
+    // the same block then ignores. So 26 product files were never linted.
+    //
+    // Stylistic rules only. eslint-config-love is type-aware and expects a
+    // tsconfig project; pointing it at plain .js files would need type info
+    // these files do not have. This at least holds the shipped code to the same
+    // formatting as everything else.
+    files: ['lib/**/*.js'],
+    plugins: { '@stylistic': stylistic },
+    rules: {
+      '@stylistic/semi': ['error', 'never'],
+      '@stylistic/no-trailing-spaces': 'error',
+      '@stylistic/eol-last': ['error', 'always'],
+      '@stylistic/no-multiple-empty-lines': ['error', { max: 1, maxEOF: 0 }],
+      '@stylistic/quotes': ['error', 'single', { avoidEscape: true }],
+      '@stylistic/comma-dangle': ['error', 'never'],
+      '@stylistic/space-before-function-paren': ['error', 'always'],
+      '@stylistic/keyword-spacing': 'error',
+      '@stylistic/space-infix-ops': 'error',
+      '@stylistic/comma-spacing': 'error',
+      '@stylistic/brace-style': ['error', '1tbs', { allowSingleLine: true }],
+      '@stylistic/block-spacing': 'error',
+      '@stylistic/key-spacing': 'error',
+      '@stylistic/space-before-blocks': 'error',
+      '@stylistic/no-multi-spaces': 'error',
+      '@stylistic/object-curly-spacing': ['error', 'always'],
+      '@stylistic/array-bracket-spacing': ['error', 'never'],
+      '@stylistic/spaced-comment': ['error', 'always'],
+      '@stylistic/indent': ['error', 2]
+    }
+  },
   {
     ignores: ['dist/**', 'node_modules/**']
   }

lib/notifier.js

@@ -222,7 +222,7 @@ const notifyModule = ((() => {
       if (this.timeoutHandle) {
         clearTimeout(this.timeoutHandle)
       }
-      
+
       if (timeoutMs && timeoutMs > 0) {
         this.timeoutHandle = setTimeout(() => {
           logger.debugLazy(() => `Timeout ${timeoutMs}ms reached for query ${this.queryId} - triggering cancel`)

lib/sql-client.js

@@ -114,9 +114,9 @@ class SqlClient {
   getConnectObject (p) {
     return typeof p === 'string'
       ? {
-          conn_str: p,
-          connect_timeout: 0
-        }
+        conn_str: p,
+        connect_timeout: 0
+      }
       : p
   }
 

package.json

@@ -96,8 +96,8 @@
     "test:cpp": "npm run configure:cpp-tests && npm run build:cpp-tests && cd test/cpp/build && ctest -C Release -V",
     "test:cpp:debug": "npm run configure:cpp-tests && cmake --build test/cpp/build --config Debug -j 8 && cd test/cpp/build && ctest -C Debug -V",
     "test:cpp:debug:exe": "test/cpp/build/cpp_tests --gtest_filter=\"*\"",
-    "lint": "eslint \"test/**/*.ts\" \"samples/**/*.ts\"",
-    "lint:fix": "eslint \"test/**/*.ts\" \"samples/**/*.ts\" --fix",
+    "lint": "eslint \"lib/**/*.js\" \"test/**/*.ts\" \"samples/**/*.ts\"",
+    "lint:fix": "eslint \"lib/**/*.js\" \"test/**/*.ts\" \"samples/**/*.ts\" --fix",
     "test:cpp:clean": "rimraf test/cpp/build && npm run configure:cpp-tests && cmake --build test/cpp/build --config Debug -j 8",
     "test:cpp:clean:vs": "rimraf test/cpp/build && npm run configure:cpp-tests:vs && cmake --build test/cpp/build --config Debug -j 8",
     "build:debug": "node-gyp rebuild --debug --verbose -j 4",

samples/typescript/logger-example.ts

@@ -17,26 +17,26 @@ sql.logger.configureForDevelopment()
 // Replace with your connection string
 const connectionString = 'Driver={ODBC Driver 18 for SQL Server};Server=127.0.0.1,1433;Database=node;UID=node_user;PWD=StrongPassword123!;TrustServerCertificate=yes;;Connect Timeout=10'
 
-async function main(): Promise<void> {
+async function main (): Promise<void> {
   let connection
 
   try {
     // The logger will automatically log connection attempts
     connection = await sql.promises.open(connectionString)
-    
+
     // Simple query
     const result: QueryAggregatorResults = await connection.promises.query(
       'SELECT name, create_date FROM sys.tables WHERE type = ?',
-      ['U']  // 'U' for user tables
+      ['U'] // 'U' for user tables
     )
-    
+
     console.log(`Found ${result.rows} tables`)
     console.log('First few tables:', result.first?.slice(0, 3))
-    
+
     // Stored procedure call (if you have one)
     // const procResult = await connection.promises.callProc('sp_who')
     // console.log('Active connections:', procResult.first?.length)
-    
+
   } catch (error) {
     // Errors are automatically logged at ERROR level
     console.error('Database operation failed:', error)
@@ -48,7 +48,7 @@ async function main(): Promise<void> {
 }
 
 // Using connection pool with logging
-async function poolExample(): Promise<void> {
+async function poolExample (): Promise<void> {
   const pool = new sql.Pool({
     connectionString,
     floor: 2,
@@ -57,16 +57,16 @@ async function poolExample(): Promise<void> {
 
   try {
     await pool.promises.open()
-    
+
     // Execute multiple queries - the pool and logger will show which connection handles each
     const results = await Promise.all([
       pool.promises.query('SELECT 1 as num'),
       pool.promises.query('SELECT 2 as num'),
       pool.promises.query('SELECT 3 as num')
     ])
-    
+
     console.log('Pool query results:', results.map(r => r.first?.[0]))
-    
+
   } finally {
     await pool.promises.close()
   }
@@ -76,11 +76,16 @@ async function poolExample(): Promise<void> {
 (async () => {
   console.log('=== Single Connection Example ===')
   await main()
-  
+
   console.log('\n=== Connection Pool Example ===')
   await poolExample()
-  
+
   // Get final logger stats
   const config = sql.logger.getConfiguration()
   console.log('\nLogger configuration:', config)
-})()
+})().catch(e => {
+  // Without this the IIFE is a floating promise: any failure above becomes
+  // an unhandled rejection, exiting with a bare stack trace and no context.
+  console.error(e)
+  process.exitCode = 1
+})

samples/typescript/pooling.ts

@@ -34,7 +34,7 @@ function submit (sql: string): Query {
   console.log(`send ${timeStr}, sql = ${sql}`)
   q.on('submitted', (d: QueryDescription) => {
     console.log(`query submitted ${timeStr}, sql = ${d.query_str}`)
-    q.on('done', () => console.log(`query done ${timeStr}`))
+    q.on('done', () => { console.log(`query done ${timeStr}`) })
   })
   return q
 }

samples/typescript/simple-logging-promises.ts

@@ -27,30 +27,30 @@ console.log('Current logger configuration:', config)
 // Example connection string - replace with your own
 const connectionString = 'Driver={ODBC Driver 18 for SQL Server};Server=127.0.0.1,1433;Database=node;UID=node_user;PWD=StrongPassword123!;TrustServerCertificate=yes;;Connect Timeout=10'
 
-async function demonstrateLogging(): Promise<void> {
+async function demonstrateLogging (): Promise<void> {
   try {
     // Log custom messages at different levels
     sql.logger.trace('Starting promise-based SQL operations', 'demonstrateLogging')
     sql.logger.debug('Connection string configured', 'demonstrateLogging')
-    
+
     // Open connection using promises
     sql.logger.info('Opening database connection...', 'demonstrateLogging')
     const connection = await sql.promises.open(connectionString)
     sql.logger.info('Connection opened successfully', 'demonstrateLogging')
-    
+
     // Execute a simple query
     sql.logger.debug('Executing test query', 'demonstrateLogging')
     const results: QueryAggregatorResults = await connection.promises.query('SELECT 1 as test, GETDATE() as currentTime')
-    
+
     sql.logger.info(`Query completed in ${results.elapsed}ms`, 'demonstrateLogging')
     sql.logger.debug(`Results: ${JSON.stringify(results.first)}`, 'demonstrateLogging')
-    
+
     // Demonstrate lazy logging for expensive operations
     sql.logger.traceLazy(() => {
       // This function is only called if TRACE level is enabled
       return `Detailed results: ${JSON.stringify(results, null, 2)}`
     }, 'demonstrateLogging')
-    
+
     // Execute multiple queries using pool
     sql.logger.info('Creating connection pool...', 'demonstrateLogging')
     const pool = new sql.Pool({
@@ -61,30 +61,30 @@ async function demonstrateLogging(): Promise<void> {
       heartbeatSql: 'SELECT @@SPID as spid',
       inactivityTimeoutSecs: 60
     })
-    
+
     await pool.promises.open()
     sql.logger.info('Pool opened successfully', 'demonstrateLogging')
-    
+
     // Execute queries in parallel
     sql.logger.debug('Executing parallel queries', 'demonstrateLogging')
     const parallelQueries = Promise.all([
       pool.promises.query('SELECT 1 as query1'),
       pool.promises.query('SELECT 2 as query2'),
       pool.promises.query('SELECT 3 as query3')
     ])
-    
+
     const parallelResults = await parallelQueries
     sql.logger.info(`Completed ${parallelResults.length} parallel queries`, 'demonstrateLogging')
-    
+
     // Clean up
     sql.logger.debug('Closing pool', 'demonstrateLogging')
     await pool.promises.close()
-    
+
     sql.logger.debug('Closing connection', 'demonstrateLogging')
     await connection.promises.close()
-    
+
     sql.logger.info('All operations completed successfully', 'demonstrateLogging')
-    
+
   } catch (error) {
     sql.logger.error(`Operation failed: ${error}`, 'demonstrateLogging')
     throw error
@@ -103,4 +103,4 @@ demonstrateLogging()
   .catch((error) => {
     console.error('Logging demonstration failed:', error)
     process.exit(1)
-  })
+  })